Skip to content

Logs url#380

Merged
brucetony merged 5 commits into
developfrom
logs-url
May 31, 2026
Merged

Logs url#380
brucetony merged 5 commits into
developfrom
logs-url

Conversation

@brucetony
Copy link
Copy Markdown
Collaborator

@brucetony brucetony commented May 31, 2026

Summary by CodeRabbit

  • New Features

    • Added configurable result limit control (default 50) when filtering events.
    • VictoriaLogs URL is now configurable via environment variable.
  • Improvements

    • Simplified analysis descriptions by removing redundant detailed criteria sections.
    • Enhanced button tooltips to accurately reflect disable conditions.
    • Improved event timestamp display with date and time on separate lines.
    • Events table now defaults to sorting by timestamp in descending order.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

Warning

Review limit reached

@brucetony, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 22 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13cf09bd-1508-41e7-8641-a7ded9791a38

📥 Commits

Reviewing files that changed from the base of the PR and between d2e0777 and 9272205.

📒 Files selected for processing (3)
  • app/components/analysis/AnalysisControlButtons.vue
  • app/components/events/DateFilterGraph.vue
  • app/components/events/EventViewer.vue
📝 Walkthrough

Walkthrough

This PR refines the analysis UI by simplifying table descriptions and aligning button disabled messaging, introduces pagination to the event viewer with a configurable query limit, and adds VictoriaLogs URL configuration to the application runtime.

Changes

Analysis UI Refinement

Layer / File(s) Summary
Analysis table and button state messaging
app/components/analysis/AnalysesTable.vue, app/components/analysis/AnalysisControlButtons.vue
AnalysesTable removes the expanded "start criteria" section from the Analyses description and updates the Approval Status column tooltip. AnalysisControlButtons introduces a disabledReason computed value that consolidates disable conditions (approval status, execution state, datastore requirement) and uses it to align play, rerun, stop, and delete button tooltips with the actual disable logic.

Event Viewer Pagination Feature

Layer / File(s) Summary
DateFilterGraph pagination component
app/components/events/DateFilterGraph.vue
DateFilterGraph script expands props to accept optional initialStartDate and initialEndDate, introduces reactive queryLimit ref (default 50), updates the applyDateFilter emit to include the limit parameter, and triggers an initial emit on mount. Template adds an InputNumber control for users to set the query limit.
EventViewer pagination and formatting
app/components/events/EventViewer.vue
EventViewer introduces reactive fetchLimit state, updates the event query payload to use the limit, extends the date filter apply handler to accept and apply the limit parameter, refactors timestamp formatting into separate formatTimestampDate() and formatTimestampTime() functions with error handling, and sets default table sorting by timestamp in descending order.
Pagination behavior tests
test/components/events/DateFilterGraph.spec.ts, test/components/events/EventViewer.spec.ts
Tests are updated to use Vitest fake timers with a fixed system time for deterministic assertions. DateFilterGraph tests verify that applyDateFilter emits on mount with a computed 5-minute window and default limit of 50, and that initialStartDate/initialEndDate props override the computed window. EventViewer tests confirm that mount-time event fetching uses correct query parameters including the limit and computed date window.

VictoriaLogs URL Configuration

Layer / File(s) Summary
VictoriaLogs URL config and menu integration
nuxt.config.ts, app/components/header/AvatarButton.vue
nuxt.config.ts adds runtimeConfig.public.victoriaLogs environment variable to public config. AvatarButton.vue reads the configured URL into victoriaLogsUrl and updates the VictoriaLogs menu item to use it for the link and to render only when the user is authenticated and the URL is non-empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PrivateAIM/node-ui#309: The main PR's pagination feature in DateFilterGraph.vue and EventViewer.vue directly evolves the event viewer components introduced in that PR with new limit parameter handling and initial date prop behavior.
  • PrivateAIM/node-ui#323: Both PRs modify AnalysisControlButtons.vue to consolidate button disabled state logic based on datastore requirements and approval status.
  • PrivateAIM/node-ui#313: Both PRs update AnalysisControlButtons.vue to refine how button disabled state and tooltips are derived from analysis run and approval status.

Poem

🐰 A rabbit hops through the code,
Paging events down the road,
Analysis buttons now shine bright,
VictoriaLogs links take flight,
Cleaner tables, smarter state—
Filtering data, oh how great! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The PR title 'Logs url' is vague and does not clearly convey the scope and intent of the changes, which include updates to multiple components (AnalysesTable, AnalysisControlButtons, DateFilterGraph, EventViewer, AvatarButton), configuration changes, and test updates. Use a more descriptive title that summarizes the main change, such as 'Add VictoriaLogs URL configuration and update event filtering' or 'Configure VictoriaLogs URL and refactor event date filtering'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch logs-url

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.

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: 3

🧹 Nitpick comments (1)
app/components/header/AvatarButton.vue (1)

17-17: ⚡ Quick win

Refine the type assertion for optional environment variable.

The as string assertion is imprecise because config.public.victoriaLogs can be undefined when the environment variable is not set. While the runtime behavior is safe (due to the !!victoriaLogsUrl check on line 50), TypeScript best practice is to reflect the actual type.

♻️ Suggested type refinement
-const victoriaLogsUrl = config.public.victoriaLogs as string;
+const victoriaLogsUrl = config.public.victoriaLogs as string | undefined;

Alternatively, remove the type assertion entirely and let TypeScript infer the type.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/components/header/AvatarButton.vue` at line 17, The variable declaration
uses a blunt "as string" on config.public.victoriaLogs even though it can be
undefined; change the type to reflect that optionality (e.g., declare
victoriaLogsUrl as string | undefined or simply remove the assertion and let
TypeScript infer the type from config.public.victoriaLogs) and keep the existing
runtime check (!!victoriaLogsUrl) in the AvatarButton.vue logic so the code
still guards against undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/components/analysis/AnalysisControlButtons.vue`:
- Around line 160-166: The tooltip text comes from the computed disabledReason
but the Stop/Delete buttons’ :disabled bindings (where they reference
notApproved) are out of sync; either add the notApproved check to those buttons’
:disabled attributes (so Stop/Delete actually disable when notApproved is true)
or split the logic: create a separate computed (e.g., playRerunDisabledReason or
playDisabled) used by Play/Rerun that includes the notApproved check and keep
disabledReason for Stop/Delete without the approval check; update the button
bindings accordingly (references: disabledReason, notApproved, and the
Stop/Delete button :disabled attributes in the template).

In `@app/components/events/DateFilterGraph.vue`:
- Around line 102-109: The InputNumber can produce null when cleared, so prevent
emitting null by coercing queryLimit to a fallback or disallowing empty input:
either add :allowEmpty="false" on the InputNumber component in
DateFilterGraph.vue or, when you emit/use queryLimit (the queryLimit ref used by
DateFilterGraph.vue and consumed by EventViewer.vue's buildQuery()), coerce it
to a safe default (e.g., const limit = queryLimit.value ?? 50) before emitting
or passing into buildQuery() to ensure { limit: null } is never produced.

In `@app/components/events/EventViewer.vue`:
- Around line 201-219: formatTimestampDate and formatTimestampTime are brittle
because they split dateTimeFormat.format(...) on a locale-dependent string;
replace them with a single locale-safe helper that either uses separate
Intl.DateTimeFormat instances (one with dateStyle and one with timeStyle) or
uses dateTimeFormat.formatToParts to extract the "date" and "time" parts
deterministically. Implement a generic formatTimestampPart(timestamp,
formatterOrPart) and remove the near-duplicate helpers, then update calls that
used formatTimestampDate/formatTimestampTime to call the new helper (referencing
the existing dateTimeFormat variable if needed or create
dateFormatter/timeFormatter and use those in formatTimestampPart).

---

Nitpick comments:
In `@app/components/header/AvatarButton.vue`:
- Line 17: The variable declaration uses a blunt "as string" on
config.public.victoriaLogs even though it can be undefined; change the type to
reflect that optionality (e.g., declare victoriaLogsUrl as string | undefined or
simply remove the assertion and let TypeScript infer the type from
config.public.victoriaLogs) and keep the existing runtime check
(!!victoriaLogsUrl) in the AvatarButton.vue logic so the code still guards
against undefined.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8bb87460-8664-4125-a262-a1705e1e5253

📥 Commits

Reviewing files that changed from the base of the PR and between 55c2b69 and d2e0777.

📒 Files selected for processing (8)
  • app/components/analysis/AnalysesTable.vue
  • app/components/analysis/AnalysisControlButtons.vue
  • app/components/events/DateFilterGraph.vue
  • app/components/events/EventViewer.vue
  • app/components/header/AvatarButton.vue
  • nuxt.config.ts
  • test/components/events/DateFilterGraph.spec.ts
  • test/components/events/EventViewer.spec.ts

Comment thread app/components/analysis/AnalysisControlButtons.vue
Comment thread app/components/events/DateFilterGraph.vue
Comment thread app/components/events/EventViewer.vue Outdated
@brucetony brucetony merged commit cf2eba1 into develop May 31, 2026
5 checks passed
@brucetony brucetony deleted the logs-url branch May 31, 2026 07:33
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