fix multiple core issues and mount admin dashboard UI#67
fix multiple core issues and mount admin dashboard UI#67Kritika11052005 wants to merge 1 commit into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a ChangesAdmin Dashboard UI
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)
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()
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
humane_proxy/classifiers/heuristics.py (1)
137-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilently swallowing all exceptions hides config load failures.
The bare
except Exceptionfalls back to the stale module-level_CFGwithout any signal, so a brokenget_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 winDuplicated auth-header logic; inconsistent 401 handling between
apiFetchanddeleteSession.Both functions independently read
hp-admin-keyand build theAuthorizationheader.deleteSessionalso skips the key-reprompt flow thatapiFetchhas 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
deleteSessioncan callapiFetch(\/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
⛔ Files ignored due to path filters (2)
humane_proxy/dashboard/public/logo.pngis excluded by!**/*.pnguv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
humane_proxy/api/admin.pyhumane_proxy/classifiers/heuristics.pyhumane_proxy/classifiers/stage3/llamaguard.pyhumane_proxy/dashboard/templates/index.htmlhumane_proxy/escalation/router.pyhumane_proxy/storage/redis.pypyproject.tomltests/test_admin_api.pytests/test_heuristics.pytests/test_router.pytests/test_stage3.pytests/test_storage_backends.py
|
@Kritika11052005 For now, create an issue for dashboard mounting, rebase onto the current main and reduce this PR to only the dashboard work.The |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
humane_proxy/api/admin.py (1)
48-72: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDashboard serving logic is well-structured.
The three-tier path resolution covers dev, packaged, and system-prefix layouts. The runtime
exists()re-check inserve_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 valueRUF012: 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, aClassVarannotation 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
📒 Files selected for processing (3)
dashboard/templates/index.htmlhumane_proxy/api/admin.pytests/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> |
There was a problem hiding this comment.
🎯 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.
| <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.
| 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); | ||
| } |
There was a problem hiding this comment.
🎯 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.
| 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.
| 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); |
There was a problem hiding this comment.
🎯 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 exposesby_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.
| 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); | ||
| }); |
There was a problem hiding this comment.
🩺 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.
Note
Please apply the
gssoc26label 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:
humane_proxy/classifiers/heuristics.pyto reload dynamically when config updates occur, preventing outdated rules from being cached."email_to"targets inhumane_proxy/escalation/router.pyto prevent webhooks from being suppressed when only email destinations are configured.S9(Indiscriminate Weapons) to map to"criminal_intent"instead of"safe"inhumane_proxy/classifiers/stage3/llamaguard.py.humane_proxy/storage/redis.pyto increment counters atomically and set key expiration times correctly.GET /adminandGET /admin/public/logo.pngto serve the HTML UI and asset logo. Updated the frontend dashboard with live stats/logs queries, authentication key prompt handling vialocalStorage, and interactive session data deletion.Type of Change
Checklist
pytest tests/ -v)