Skip to content

fix multiple core issues and mount admin dashboard UI#67

Open
Kritika11052005 wants to merge 1 commit into
Vishisht16:mainfrom
Kritika11052005:main
Open

fix multiple core issues and mount admin dashboard UI#67
Kritika11052005 wants to merge 1 commit into
Vishisht16:mainfrom
Kritika11052005:main

Conversation

@Kritika11052005

Copy link
Copy Markdown

Note

Please apply the gssoc26 label to this Pull Request so it can be tracked for my contributions in GirlScript Summer of Code 2026.

Description

This PR resolves four critical pipeline, safety, and storage bugs, and fully integrates the FastAPI Admin Dashboard UI.

Summary of Changes:

  1. Dynamic Heuristics Configuration: Updated the safety compilation logic in humane_proxy/classifiers/heuristics.py to reload dynamically when config updates occur, preventing outdated rules from being cached.
  2. Email Webhooks Alerting: Fixed extraction paths for "email_to" targets in humane_proxy/escalation/router.py to prevent webhooks from being suppressed when only email destinations are configured.
  3. LlamaGuard S9 Mapping: Corrected category S9 (Indiscriminate Weapons) to map to "criminal_intent" instead of "safe" in humane_proxy/classifiers/stage3/llamaguard.py.
  4. Redis Rate Limiter: Fixed the rate limit session counting logic in humane_proxy/storage/redis.py to increment counters atomically and set key expiration times correctly.
  5. FastAPI Admin Dashboard: Mounted API endpoints at GET /admin and GET /admin/public/logo.png to serve the HTML UI and asset logo. Updated the frontend dashboard with live stats/logs queries, authentication key prompt handling via localStorage, and interactive session data deletion.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Checklist

  • I have read the CONTRIBUTING guide
  • My code follows the project's style
  • I have added tests for new or changed behaviour
  • All tests pass (pytest tests/ -v)
  • I have updated documentation if needed
  • Self-harm / safety-related changes have been reviewed for sensitivity

@CLAassistant

CLAassistant commented Jul 3, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Admin dashboard now loads directly in the browser at /admin and /admin/.
    • Added admin key sign-in for accessing dashboard data.
    • Dashboard now shows live health, stats, escalation details, and chart updates.
    • Added per-escalation delete actions with confirmation.
  • Bug Fixes

    • Improved dashboard loading and fallback behavior when the page can’t be found.
    • Updated chart rendering for more reliable display.

Walkthrough

Adds a serve_dashboard() route in humane_proxy/api/admin.py that locates and serves dashboard/templates/index.html for GET /admin and /admin/, with 404 handling if missing. The dashboard template gains an admin-key entry button, DOM-updatable metric cards, an Actions column with delete controls, refactored chart-drawing logic with safety guards, and authenticated live data loading (apiFetch, loadData, deleteSession). Tests cover the new dashboard endpoints.

Changes

Admin Dashboard UI

Layer / File(s) Summary
Dashboard serving endpoint
humane_proxy/api/admin.py, tests/test_admin_api.py
Adds HTMLResponse/Path imports, resolves candidate paths for index.html, adds serve_dashboard() for GET /admin and /admin/ with 404 on missing file, and adds TestDashboard tests asserting 200 and expected content.
Dashboard header, metrics, and table markup
dashboard/templates/index.html
Adds a hidden #auth-btn header button, gives metric cards stable DOM IDs, and adds an Actions column with per-row Delete controls to the escalations table.
Chart rendering refactor and safety fixes
dashboard/templates/index.html
Refactors risk/bar/donut drawing to use named data arrays, guards against invalid division/indexing in risk point calculation, adjusts polygon closing logic, expands the donut palette, and derives center totals dynamically.
Authenticated data loading and session deletion
dashboard/templates/index.html
Adds apiFetch (Bearer auth, 401/403 key prompt), escapeHtml, deleteSession, and loadData (fetches health/stats/escalations, updates metrics, rebuilds table, animates charts), with click handlers wired for delete buttons and the auth-key button.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant AdminAPI
  participant Filesystem
  Browser->>AdminAPI: GET /admin
  AdminAPI->>Filesystem: locate index.html
  Filesystem-->>AdminAPI: file found
  AdminAPI-->>Browser: HTMLResponse(index.html)
Loading
sequenceDiagram
  participant Browser
  participant apiFetch
  participant AdminAPI
  Browser->>apiFetch: loadData()
  apiFetch->>AdminAPI: GET /admin/health, /admin/stats, /admin/escalations
  AdminAPI-->>apiFetch: JSON data
  apiFetch-->>Browser: update DOM metrics, rebuild table, animateCharts()
  Browser->>apiFetch: deleteSession(sessionId)
  apiFetch->>AdminAPI: DELETE /admin/sessions/id
  AdminAPI-->>apiFetch: success/failure
  apiFetch-->>Browser: alert + loadData()
Loading

Possibly related PRs

  • Vishisht16/Humane-Proxy#65: Introduced the static dashboard template that this PR extends with authenticated live data loading and delete actions.

Suggested labels: type:design, type:feature

Suggested reviewers: Vishisht16

Poem

A rabbit hops to /admin gate,
The dashboard blooms in HTML state,
Charts now dance with safer math,
A key unlocks the data path,
Delete a session, reload anew—
Thump thump! The board updates for you. 🐇📊

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed It clearly covers the dashboard UI mount, though it also mentions broader core fixes not shown in the diff.
Description check ✅ Passed The description is related to the dashboard work and the broader bug-fix changes, so it passes the lenient check.
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.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
humane_proxy/classifiers/heuristics.py (1)

137-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Silently swallowing all exceptions hides config load failures.

The bare except Exception falls back to the stale module-level _CFG without any signal, so a broken get_config() (bad YAML, etc.) degrades silently. Consider narrowing and at least logging at warning level.

♻️ Suggested change
     try:
         current_config = humane_proxy.config.get_config().get("heuristics", {})
-    except Exception:
+    except Exception:  # noqa: BLE001 - degrade to last-known-good config
+        logger.warning("Failed to load heuristics config; using cached defaults", exc_info=True)
         current_config = _CFG
🤖 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 `@humane_proxy/classifiers/heuristics.py` around lines 137 - 140, The
heuristics config lookup in get_config() is swallowing all failures and silently
falling back to the stale module-level _CFG. Update the try/except around
humane_proxy.config.get_config() in heuristics.py to catch only expected
config-loading errors, and add a warning-level log that includes the exception
details before falling back so broken config sources are visible.

Source: Linters/SAST tools

humane_proxy/dashboard/templates/index.html (1)

724-773: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated auth-header logic; inconsistent 401 handling between apiFetch and deleteSession.

Both functions independently read hp-admin-key and build the Authorization header. deleteSession also skips the key-reprompt flow that apiFetch has on 401/403, so a stale/missing key during deletion just shows a generic failure alert instead of letting the admin re-enter the key.

♻️ Proposed consolidation
-        // API Integration
-        async function apiFetch(endpoint) {
-            const key = localStorage.getItem('hp-admin-key') || '';
-            const headers = {};
-            if (key) {
-                headers['Authorization'] = `Bearer ${key}`;
-            }
-            const res = await fetch(endpoint, { headers });
+        // API Integration
+        function authHeaders() {
+            const key = localStorage.getItem('hp-admin-key') || '';
+            return key ? { Authorization: `Bearer ${key}` } : {};
+        }
+
+        async function apiFetch(endpoint, options = {}) {
+            const res = await fetch(endpoint, { ...options, headers: { ...authHeaders(), ...(options.headers || {}) } });
             if (res.status === 401 || res.status === 403) {
                 const newKey = prompt("HumaneProxy Admin Key is required or invalid. Please enter the key:");
                 if (newKey) {
                     localStorage.setItem('hp-admin-key', newKey.trim());
                     location.reload();
                 }
+                throw new Error(`API error: ${res.status}`);
             }
             if (!res.ok) {
                 throw new Error(`API error: ${res.status}`);
             }
-            return res.json();
+            return options.method === 'DELETE' ? res : res.json();
         }

Then deleteSession can call apiFetch(\/admin/sessions/${sessionId}`, { method: 'DELETE' })` instead of duplicating header logic.

🤖 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 `@humane_proxy/dashboard/templates/index.html` around lines 724 - 773, The
auth-header setup is duplicated between apiFetch and deleteSession, and
deleteSession does not reuse the 401/403 re-prompt flow. Refactor apiFetch so it
can accept request options like method and call it from deleteSession for the
DELETE request, letting apiFetch own the hp-admin-key retrieval, Authorization
header construction, and invalid-key retry behavior. Keep deleteSession focused
on confirmation and success/error handling, and remove its local header-building
logic.
🤖 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 `@humane_proxy/dashboard/templates/index.html`:
- Around line 745-748: The delete button currently injects item.session_id into
an onclick JS string, which is vulnerable to quote-based XSS because
escapeHtml() only protects HTML text, not JavaScript-string context. Update the
dashboard template around the delete button rendering and the deleteSession()
wiring to avoid inline onclick entirely: store the session ID in a
data-session-id attribute and handle clicks with a delegated event listener that
reads the dataset value. Keep escapeHtml() for HTML text only, and ensure any
session_id usage in the template is safely passed through the new
attribute-based flow.

In `@tests/test_router.py`:
- Around line 27-40: The test currently mocks out the function that contains the
`has_any` email-detection logic, so it never verifies the fix. Update
`test_fire_webhooks_only_email` to exercise `_fire_webhooks` or
`dispatch_webhooks` directly with an email-only config, mock only `get_config`
and the lower-level dispatcher, and assert that dispatch actually happens for
the `email` webhook instead of only checking that `escalate()` calls
`_fire_webhooks()`.

---

Nitpick comments:
In `@humane_proxy/classifiers/heuristics.py`:
- Around line 137-140: The heuristics config lookup in get_config() is
swallowing all failures and silently falling back to the stale module-level
_CFG. Update the try/except around humane_proxy.config.get_config() in
heuristics.py to catch only expected config-loading errors, and add a
warning-level log that includes the exception details before falling back so
broken config sources are visible.

In `@humane_proxy/dashboard/templates/index.html`:
- Around line 724-773: The auth-header setup is duplicated between apiFetch and
deleteSession, and deleteSession does not reuse the 401/403 re-prompt flow.
Refactor apiFetch so it can accept request options like method and call it from
deleteSession for the DELETE request, letting apiFetch own the hp-admin-key
retrieval, Authorization header construction, and invalid-key retry behavior.
Keep deleteSession focused on confirmation and success/error handling, and
remove its local header-building logic.
🪄 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: be38893f-59eb-48cf-acd5-d727932d1648

📥 Commits

Reviewing files that changed from the base of the PR and between 111e480 and 3df5fc1.

⛔ Files ignored due to path filters (2)
  • humane_proxy/dashboard/public/logo.png is excluded by !**/*.png
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (12)
  • humane_proxy/api/admin.py
  • humane_proxy/classifiers/heuristics.py
  • humane_proxy/classifiers/stage3/llamaguard.py
  • humane_proxy/dashboard/templates/index.html
  • humane_proxy/escalation/router.py
  • humane_proxy/storage/redis.py
  • pyproject.toml
  • tests/test_admin_api.py
  • tests/test_heuristics.py
  • tests/test_router.py
  • tests/test_stage3.py
  • tests/test_storage_backends.py

Comment thread humane_proxy/dashboard/templates/index.html Outdated
Comment thread tests/test_router.py Outdated
@Vishisht16

Copy link
Copy Markdown
Owner

@Kritika11052005
Due to this being a stray PR without an explicit issue assigned to you, I didn't review the PR earlier. Most of the fixes you added were implemented in the latest bug-fix series.
As per our CONTRIBUTING.md, you must first get an issue assigned before opening a PR to avoid duplicate work. Unassigned PRs are hard to review fairly and I may close them without review in the future.

For now, create an issue for dashboard mounting, rebase onto the current main and reduce this PR to only the dashboard work.The dashboard/ will live in the repo root.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 (2)
humane_proxy/api/admin.py (1)

48-72: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Dashboard serving logic is well-structured.

The three-tier path resolution covers dev, packaged, and system-prefix layouts. The runtime exists() re-check in serve_dashboard() correctly handles the case where the file is removed after import.

One minor consideration: _HTML_PATH.read_text() reads the file from disk on every request. For an admin dashboard this is acceptable, but if request volume grows, consider caching the content at import time (or with a simple mtime check) to avoid repeated I/O.

🤖 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 `@humane_proxy/api/admin.py` around lines 48 - 72, serve_dashboard currently
reads the dashboard template from disk on every request via
_HTML_PATH.read_text(), which adds unnecessary I/O. Update the admin dashboard
serving flow in humane_proxy/api/admin.py by caching the HTML content once (for
example at import time alongside _HTML_PATH, or with a lightweight mtime-based
refresh) and have serve_dashboard return the cached content while preserving the
existing not-found check.
tests/test_admin_api.py (1)

166-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

RUF012: Mutable class-level dict attributes.

Ruff flags HEADERS = {"Authorization": ...} as a mutable default class attribute (RUF012). This follows the existing pattern at line 155, so it's consistent within the file. If you want to silence the warning, a ClassVar annotation or a module-level constant would work, but this is purely cosmetic for test code.

Also applies to: 185-185

🤖 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 `@tests/test_admin_api.py` at line 166, The HEADERS constant is a mutable
class-level dict, which triggers RUF012 in the test class. Update the HEADERS
attribute used in the admin API tests to follow a non-mutable class constant
pattern, either by annotating it as a ClassVar or moving it to a module-level
constant and reusing it where needed, keeping it consistent with the existing
test file pattern.

Source: Linters/SAST tools

🤖 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 `@dashboard/templates/index.html`:
- Around line 781-790: The dashboard metrics are showing values that do not
match their labels, so update the stats rendering logic in the template script
that sets stat-escalations-today and stat-total-messages. Use the correct source
for the “Escalations Today” card by deriving today’s count from the stats
payload’s by_day data instead of stats.total_escalations, and change the “Total
Messages” card to either use an actual message-count field if available or
relabel the card to reflect escalation totals. Keep the existing unique
references around apiFetch('/admin/escalations?limit=100'), stats, items, and
the DOM ids so the fix stays localized.
- Around line 775-782: The `stat-escalations-today` update is incorrectly nested
inside the `stats.by_category` guard in the dashboard template, so the total
escalations card can stay stale when categories are missing. Update the logic
around the `stats.by_category` handling so
`document.getElementById('stat-escalations-today').textContent` is set whenever
`stats.total_escalations` is available, regardless of whether `by_category`
exists. Keep the donut chart population inside the category check, but move the
metric write outside it in the same script block.
- Line 144: The auth button label contains mojibake instead of the intended key
icon. Update the button text in the header markup so the corrupted glyph in the
auth button is replaced with the correct key emoji/character, keeping the
existing button structure and styling intact. Use the auth button element as the
target for the fix.
- Around line 809-820: Guard the row rendering in the table rebuild so missing
or invalid data does not abort the entire loop: in the `tbody` build logic that
sets `tr.innerHTML`, sanitize `item.risk_score` before calling `toFixed(2)` and
validate `item.timestamp` before formatting it into `dateStr`, so `forEach` can
continue rendering the remaining rows even when one record is malformed. Update
the formatting logic around `item.risk_score`, `item.timestamp`, and the
`tr.innerHTML` assignment to use safe fallbacks instead of assuming both fields
are always present and valid.

---

Nitpick comments:
In `@humane_proxy/api/admin.py`:
- Around line 48-72: serve_dashboard currently reads the dashboard template from
disk on every request via _HTML_PATH.read_text(), which adds unnecessary I/O.
Update the admin dashboard serving flow in humane_proxy/api/admin.py by caching
the HTML content once (for example at import time alongside _HTML_PATH, or with
a lightweight mtime-based refresh) and have serve_dashboard return the cached
content while preserving the existing not-found check.

In `@tests/test_admin_api.py`:
- Line 166: The HEADERS constant is a mutable class-level dict, which triggers
RUF012 in the test class. Update the HEADERS attribute used in the admin API
tests to follow a non-mutable class constant pattern, either by annotating it as
a ClassVar or moving it to a module-level constant and reusing it where needed,
keeping it consistent with the existing test file pattern.
🪄 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: 08f59682-8581-4500-8b5e-3465534c989b

📥 Commits

Reviewing files that changed from the base of the PR and between 3df5fc1 and 6ce3f52.

📒 Files selected for processing (3)
  • dashboard/templates/index.html
  • humane_proxy/api/admin.py
  • tests/test_admin_api.py

<div
class="hidden rounded-full border border-zinc-200 px-3 py-1 text-xs font-medium text-zinc-500 dark:border-white/10 dark:text-zinc-400 sm:block">
admin.local</div>
<button id="auth-btn" class="hidden rounded-full border border-zinc-200 bg-zinc-50 px-3 py-1 text-xs font-semibold hover:bg-zinc-100 dark:border-white/10 dark:bg-white/[0.04] dark:hover:bg-white/[0.08] sm:block">🔑 Key</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Corrupted key-icon glyph in the auth button label.

≡ƒöæ Key is mojibake — a 🔑 emoji that was mis-encoded (UTF-8 bytes read as CP1252). It will render as garbage in the header button.

🔤 Proposed fix
-                <button id="auth-btn" class="hidden rounded-full border border-zinc-200 bg-zinc-50 px-3 py-1 text-xs font-semibold hover:bg-zinc-100 dark:border-white/10 dark:bg-white/[0.04] dark:hover:bg-white/[0.08] sm:block">🔑 Key</button>
+                <button id="auth-btn" class="hidden rounded-full border border-zinc-200 bg-zinc-50 px-3 py-1 text-xs font-semibold hover:bg-zinc-100 dark:border-white/10 dark:bg-white/[0.04] dark:hover:bg-white/[0.08] sm:block">🔑 Key</button>
📝 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
<button id="auth-btn" class="hidden rounded-full border border-zinc-200 bg-zinc-50 px-3 py-1 text-xs font-semibold hover:bg-zinc-100 dark:border-white/10 dark:bg-white/[0.04] dark:hover:bg-white/[0.08] sm:block">🔑 Key</button>
<button id="auth-btn" class="hidden rounded-full border border-zinc-200 bg-zinc-50 px-3 py-1 text-xs font-semibold hover:bg-zinc-100 dark:border-white/10 dark:bg-white/[0.04] dark:hover:bg-white/[0.08] sm:block">🔑 Key</button>
🤖 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 `@dashboard/templates/index.html` at line 144, The auth button label contains
mojibake instead of the intended key icon. Update the button text in the header
markup so the corrupted glyph in the auth button is replaced with the correct
key emoji/character, keeping the existing button structure and styling intact.
Use the auth button element as the target for the fix.

Comment on lines +775 to +782
if (stats.by_category) {
const keys = Object.keys(stats.by_category);
if (keys.length > 0) {
donutItems = keys.map(k => stats.by_category[k]);
donutTotalVal = String(stats.total_escalations || 0);
}
document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

stat-escalations-today only updates when by_category is present.

The #stat-escalations-today write on Line 781 is nested inside the if (stats.by_category) block. When stats.by_category is empty or absent (e.g. no escalations yet), the card silently keeps the hardcoded 23 sample value even though a valid total_escalations is available. Move the metric update out of the by_category guard.

🔧 Proposed fix
-                if (stats.by_category) {
-                    const keys = Object.keys(stats.by_category);
-                    if (keys.length > 0) {
-                        donutItems = keys.map(k => stats.by_category[k]);
-                        donutTotalVal = String(stats.total_escalations || 0);
-                    }
-                    document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
-                }
+                if (stats.by_category) {
+                    const keys = Object.keys(stats.by_category);
+                    if (keys.length > 0) {
+                        donutItems = keys.map(k => stats.by_category[k]);
+                        donutTotalVal = String(stats.total_escalations || 0);
+                    }
+                }
+                document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
📝 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
if (stats.by_category) {
const keys = Object.keys(stats.by_category);
if (keys.length > 0) {
donutItems = keys.map(k => stats.by_category[k]);
donutTotalVal = String(stats.total_escalations || 0);
}
document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
}
if (stats.by_category) {
const keys = Object.keys(stats.by_category);
if (keys.length > 0) {
donutItems = keys.map(k => stats.by_category[k]);
donutTotalVal = String(stats.total_escalations || 0);
}
}
document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
🤖 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 `@dashboard/templates/index.html` around lines 775 - 782, The
`stat-escalations-today` update is incorrectly nested inside the
`stats.by_category` guard in the dashboard template, so the total escalations
card can stay stale when categories are missing. Update the logic around the
`stats.by_category` handling so
`document.getElementById('stat-escalations-today').textContent` is set whenever
`stats.total_escalations` is available, regardless of whether `by_category`
exists. Keep the donut chart population inside the category check, but move the
metric write outside it in the same script block.

Comment on lines +781 to +790
document.getElementById('stat-escalations-today').textContent = String(stats.total_escalations || 0);
}

const escData = await apiFetch('/admin/escalations?limit=100');
const items = escData.items || [];

document.getElementById('stat-total-messages').textContent = String(stats.total_escalations || items.length);

const uniqueSessions = new Set(items.map(i => i.session_id));
document.getElementById('stat-active-sessions').textContent = String(uniqueSessions.size);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Metric labels don't match the data being shown.

Two cards display values that don't correspond to their labels:

  • "Escalations Today" is set to stats.total_escalations (Line 781), which is the all‑time total, not today's count. The stats payload exposes by_day, which is a better source for a "today" figure.
  • "Total Messages" is set to stats.total_escalations || items.length (Line 787), i.e. an escalation count rather than a message count. The stats endpoint has no total-messages field.

Consider deriving these from the intended fields or relabeling the cards to avoid misleading operators.

🤖 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 `@dashboard/templates/index.html` around lines 781 - 790, The dashboard metrics
are showing values that do not match their labels, so update the stats rendering
logic in the template script that sets stat-escalations-today and
stat-total-messages. Use the correct source for the “Escalations Today” card by
deriving today’s count from the stats payload’s by_day data instead of
stats.total_escalations, and change the “Total Messages” card to either use an
actual message-count field if available or relabel the card to reflect
escalation totals. Keep the existing unique references around
apiFetch('/admin/escalations?limit=100'), stats, items, and the DOM ids so the
fix stays localized.

Comment on lines +809 to +820
tr.innerHTML = `
<td class="px-5 py-4 font-mono text-xs text-zinc-500">${dateStr}</td>
<td class="px-5 py-4 font-mono font-semibold">${escapeHtml(item.session_id)}</td>
<td class="px-5 py-4"><span class="rounded-full px-2.5 py-1 text-xs font-bold ${badgeColor}">${escapeHtml(item.category)}</span></td>
<td class="px-5 py-4 font-mono font-bold">${item.risk_score.toFixed(2)}</td>
<td class="px-5 py-4 text-zinc-500">${escapeHtml(triggersStr)}</td>
<td class="px-5 py-4 text-right">
<button data-session-id="${escapeHtml(item.session_id)}" class="delete-btn text-xs font-bold text-red-500 hover:text-red-700 bg-red-500/10 hover:bg-red-500/20 px-2 py-1 rounded">Delete</button>
</td>
`;
tbody.appendChild(tr);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unguarded risk_score.toFixed(2) can abort the whole table rebuild.

If any item is missing risk_score (null/undefined), item.risk_score.toFixed(2) on Line 813 throws mid-forEach, which propagates to the outer catch and leaves the table partially built (or empty) rather than rendering the remaining rows. item.timestamp on Line 799 has the same fragility (Invalid Date → toISOString() throws). Coerce/guard these fields before formatting.

🛡️ Proposed guard
-                        const d = new Date(item.timestamp * 1000);
-                        const dateStr = d.toISOString().replace('T', ' ').substring(0, 16);
+                        const d = new Date((item.timestamp || 0) * 1000);
+                        const dateStr = Number.isNaN(d.getTime())
+                            ? '—'
+                            : d.toISOString().replace('T', ' ').substring(0, 16);
-                            <td class="px-5 py-4 font-mono font-bold">${item.risk_score.toFixed(2)}</td>
+                            <td class="px-5 py-4 font-mono font-bold">${Number(item.risk_score ?? 0).toFixed(2)}</td>
🤖 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 `@dashboard/templates/index.html` around lines 809 - 820, Guard the row
rendering in the table rebuild so missing or invalid data does not abort the
entire loop: in the `tbody` build logic that sets `tr.innerHTML`, sanitize
`item.risk_score` before calling `toFixed(2)` and validate `item.timestamp`
before formatting it into `dateStr`, so `forEach` can continue rendering the
remaining rows even when one record is malformed. Update the formatting logic
around `item.risk_score`, `item.timestamp`, and the `tr.innerHTML` assignment to
use safe fallbacks instead of assuming both fields are always present and valid.

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.

3 participants