fix: SystemView vector metrics disconnected guard#31
Conversation
T003: Added v-if/v-else guard so .toFixed() is not called on undefined
when vector client returns {enabled: false} instead of numeric metrics.
T001-T002: Already implemented — pgvector instrumentation + handler use real data.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the SystemView by introducing robust error handling for vector metrics. It ensures that the user interface gracefully handles scenarios where the vector database is disconnected, preventing display errors and providing clear feedback to the user about the connection status, thereby improving the overall stability and user experience of the system monitoring page. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Пошаговое описаниеКомпонент Изменения
Оцененная трудоёмкость рецензирования🎯 3 (Moderate) | ⏱️ ~20 минут Стихотворение
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
@coderabbitai review |
|
@gemini-code-assist review |
✅ Actions performedReview triggered.
|
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request correctly adds a guard to prevent a TypeError when vector metrics are disabled, which is a good fix. The use of v-if/v-else is appropriate for this. I have added one suggestion to further improve the robustness of the component against potentially missing data from the API, even when metrics are enabled, by handling potential null or undefined values before calling .toFixed().
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/views/SystemView.vue`:
- Around line 229-262: Create a discriminated union for VectorMetrics with a
boolean literal field enabled: one variant { enabled: true; query_count: number;
avg_latency_ms: number; p50_latency_ms: number; p95_latency_ms: number;
p99_latency_ms: number; total_documents: number; uptime: string } and the other
{ enabled: false; query_count?: number; avg_latency_ms?: undefined;
p50_latency_ms?: undefined; p95_latency_ms?: undefined; p99_latency_ms?:
undefined; total_documents?: number; uptime?: string } so the type encodes that
numeric latency fields exist only when enabled is true; then update usages in
SystemView.vue (this template) and Sidebar.vue to narrow on
vectorMetrics.enabled before calling .toFixed(1) or accessing latency fields
(e.g., wrap latency displays in an if/ternary that checks enabled or guard with
a type-narrowing branch) so TypeScript enforces safe access to the numeric
fields.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8dfdca97-27ca-4c7d-8edd-020e79aa4099
📒 Files selected for processing (1)
ui/src/views/SystemView.vue
There was a problem hiding this comment.
Code Review
This pull request correctly adds a guard to the vector metrics display in SystemView.vue to prevent a TypeError when the vector database is not connected. The change checks vectorMetrics.enabled and shows a message instead of trying to render metrics with undefined values. I've added one suggestion to further improve the robustness of the component by defensively handling cases where latency values might be null even when metrics are enabled.
Root cause: HTTP WriteTimeout=60s (set by PR #31 security hardening) closes idle SSE connections, EventSource fires onerror, dashboard shows banner, client reconnects — repeat every minute. Fix: SSE handler now sends a comment-line keepalive ': keepalive\n\n' every 25s. Comments are ignored by the client but keep the HTTP write path active, preventing WriteTimeout from firing on idle streams. Also added: - X-Accel-Buffering: no header to disable nginx/reverse-proxy buffering - Client.WriteMu mutex to serialize concurrent writes between broadcast goroutines and the keepalive ticker - Initial connection message now goes through the mutex too Verified: all SSE tests pass. Dashboard banner should stay hidden when connection is healthy. Bump plugins to 3.5.7.
Backend: - Migration 075: add type column (TEXT, default 'task') to issues table - Issue model: Type field with validation (bug/feature/improvement/task) - Store: CreateIssue validates type, UpdateIssueFields accepts type, ListIssuesEx filters by type - MCP tool: type param in issues(action="create") - REST: type in POST/PATCH body, GET query param Dashboard: - "New Issue" button + creation modal (title, body, type, priority, target_project) - Type filter buttons (All/Bug/Feature/Improvement/Task) - Type badge in issue list (colored: red/blue/green/gray) - Type badge + edit in issue detail view Hooks: - formatIssuesBlock shows [TYPE] tag in session injection Plugin v3.7.8. Closes #31, #14.
* feat(issues): add type field (bug/feature/improvement/task) [T001-T008] * feat(issues): add type field + operator create from dashboard (#31, #14) Backend: - Migration 075: add type column (TEXT, default 'task') to issues table - Issue model: Type field with validation (bug/feature/improvement/task) - Store: CreateIssue validates type, UpdateIssueFields accepts type, ListIssuesEx filters by type - MCP tool: type param in issues(action="create") - REST: type in POST/PATCH body, GET query param Dashboard: - "New Issue" button + creation modal (title, body, type, priority, target_project) - Type filter buttons (All/Bug/Feature/Improvement/Task) - Type badge in issue list (colored: red/blue/green/gray) - Type badge + edit in issue detail view Hooks: - formatIssuesBlock shows [TYPE] tag in session injection Plugin v3.7.8. Closes #31, #14. * fix(issues): address PR #145 review findings - migrations.go: add DB-level CHECK constraint issues_type_check for type IN ('bug','feature','improvement','task') with proper rollback - models.go: add not null to GORM tag for Type field to match migration - handlers_issues.go: normalize type param with TrimSpace+ToLower in list, create, and update handlers - api.ts: refactor createIssue to use postJson helper (removes duplicated timeout/abort/retry logic); require non-empty target_project and validate eagerly before HTTP call - IssuesView.vue: pass target_project as string (not undefined) since createIssue now validates it directly --------- Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Summary
Changes
SystemView.vue: Wrapped metric display inv-if="vectorMetrics.enabled"guard.toFixed()TypeError on undefined valuesTest plan
Summary by CodeRabbit