UN-1798 [FIX] dashboard metrics and restore sidebar navigation#1813
Merged
Deepak-Kesavan merged 3 commits intomainfrom Mar 2, 2026
Merged
UN-1798 [FIX] dashboard metrics and restore sidebar navigation#1813Deepak-Kesavan merged 3 commits intomainfrom
Deepak-Kesavan merged 3 commits intomainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Knowledge base: Disabled due to 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbitRelease Notes
WalkthroughBackend adds HITL metrics (reviews and completions), resolves organization identifiers to stored string IDs, and safely locates the HITLQueue model; metrics are exposed via the live_series API. Frontend adds a Dashboard navigation item with a "New" tag. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client (UI)
participant Server as API Server (views/services)
participant DB as Database (Organization, HITLQueue)
Client->>Server: GET /{org}/metrics/live_series?metrics=...,hitl_reviews,hitl_completions
Server->>DB: Resolve Organization by org identifier (string)
alt Organization not found
DB-->>Server: No organization
Server-->>Client: Return empty/zero metrics for HITL
else Organization found
Server->>Server: _get_hitl_queue_model()
alt HITLQueue model unavailable
Server-->>Client: Return empty/zero metrics for HITL
else HITLQueue model available
Server->>DB: Query HITLQueue (count reviews per period by created_at)
DB-->>Server: Reviews counts per period
Server->>DB: Query HITLQueue (count APPROVED using approved_at)
DB-->>Server: Completions counts per period
Server-->>Client: Return combined metrics (including hitl_reviews, hitl_completions totals)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 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)
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 |
athul-rs
force-pushed
the
UN-1798-dashboard-fixes
branch
from
c188f0a to
15f6200
Compare
vishnuszipstack
approved these changes
Mar 2, 2026
Contributor
vishnuszipstack
left a comment
vishnuszipstack
left a comment
There was a problem hiding this comment.
move the inline css to class. other than that LGTM
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/dashboard_metrics/services.py (1)
28-39: Consider moving return to else block (static analysis hint).The static analysis tool suggests restructuring for slightly better exception handling style (TRY300):
♻️ Optional style improvement
def _get_hitl_queue_model(): """Get HITLQueue model if available (cloud-only). Returns None on OSS where manual_review_v2 is not installed. """ try: from pluggable_apps.manual_review_v2.models import HITLQueue - - return HITLQueue except ImportError: return None + else: + return HITLQueueThis is a minor style preference and the current code functions correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dashboard_metrics/services.py` around lines 28 - 39, The function _get_hitl_queue_model currently returns inside the try block which triggers a static analysis style hint; refactor it so the try block only imports and assigns HITLQueue (e.g., temp variable), use an else clause to return the model, and keep the except ImportError returning None—this removes the direct return from the try and satisfies the TRY300 style rule while preserving behavior.frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
718-731: Consider extracting inline styles to a CSS class.The inline styles work correctly, but for consistency with the rest of the component, consider moving these to
SideNavBar.css:.sidebar-item-tag { margin-left: 6px; font-size: 10px; line-height: 16px; padding: 0 4px; border-radius: 4px; }This is a minor maintainability improvement and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx` around lines 718 - 731, The inline style block applied to the Tag element inside SideNavBar.jsx (the JSX that renders {el.tag && <Tag ...>{el.tag}</Tag>}) should be moved into a CSS class for consistency: add a .sidebar-item-tag rule with the provided properties to SideNavBar.css, then replace the Tag's style prop with className="sidebar-item-tag" (keeping color="blue" and the conditional rendering intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/dashboard_metrics/services.py`:
- Around line 28-39: The function _get_hitl_queue_model currently returns inside
the try block which triggers a static analysis style hint; refactor it so the
try block only imports and assigns HITLQueue (e.g., temp variable), use an else
clause to return the model, and keep the except ImportError returning None—this
removes the direct return from the try and satisfies the TRY300 style rule while
preserving behavior.
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 718-731: The inline style block applied to the Tag element inside
SideNavBar.jsx (the JSX that renders {el.tag && <Tag ...>{el.tag}</Tag>}) should
be moved into a CSS class for consistency: add a .sidebar-item-tag rule with the
provided properties to SideNavBar.css, then replace the Tag's style prop with
className="sidebar-item-tag" (keeping color="blue" and the conditional rendering
intact).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
backend/dashboard_metrics/services.pybackend/dashboard_metrics/views.pyfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
- Fix HITL import path: manual_review_v2 → pluggable_apps.manual_review_v2 - Fix HITL model: ManualReviewEntity → HITLQueue with _base_manager - Fix pages_processed: resolve org string identifier from UUID for PageUsage - Add HITL metrics to live_series endpoint and summary - Restore metrics dashboard sidebar nav removed by sidebar refactor (#1768) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
athul-rs
force-pushed
the
UN-1798-dashboard-fixes
branch
from
6b2d28e to
6a7d9d6
Compare
for more information, see https://pre-commit.ci
chandrasekharan-zipstack
approved these changes
Mar 2, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/dashboard_metrics/services.py (1)
33-38: Optional: adopt Ruff TRY300 structure in_get_hitl_queue_model.Low-impact readability cleanup: move
return HITLQueueto anelseblock aftertry/except.♻️ Suggested diff
def _get_hitl_queue_model(): """Get HITLQueue model if available (cloud-only). Returns None on OSS where manual_review_v2 is not installed. """ try: from pluggable_apps.manual_review_v2.models import HITLQueue - - return HITLQueue except ImportError: return None + else: + return HITLQueue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dashboard_metrics/services.py` around lines 33 - 38, The helper _get_hitl_queue_model currently returns HITLQueue inside the try block; to follow Ruff TRY300 style and improve readability, change the structure so the try only imports, the except returns None, and place the return HITLQueue in an else block (or after the try/except) — locate the _get_hitl_queue_model function and the HITLQueue import reference and move the return out of the try into an else/after block so ImportError still returns None and successful imports return the model cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/dashboard_metrics/services.py`:
- Around line 33-38: The helper _get_hitl_queue_model currently returns
HITLQueue inside the try block; to follow Ruff TRY300 style and improve
readability, change the structure so the try only imports, the except returns
None, and place the return HITLQueue in an else block (or after the try/except)
— locate the _get_hitl_queue_model function and the HITLQueue import reference
and move the return out of the try into an else/after block so ImportError still
returns None and successful imports return the model cleanly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
backend/dashboard_metrics/services.pybackend/dashboard_metrics/views.pyfrontend/src/components/navigations/side-nav-bar/SideNavBar.cssfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/dashboard_metrics/views.py
- frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
muhammad-ali-e
approved these changes
Mar 2, 2026
athul-rs
changed the title
Deepak-Kesavan
approved these changes
Mar 2, 2026
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
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
5 participants
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.
What
Why
PageUsage.organization_idstores the org string identifier (e.g."org_default"), butget_pages_processed()was querying with the UUID PK — never matching any records. The backfill command stored zeros into the aggregated tables, and the/overview/endpoint served those zeros.How
get_pages_processed, resolve UUID toorganization.organization_idstring before querying PageUsageTagcomponent and renderel.tagas a badge next to the title