Add header search, Seerr request link, and alert bell (phase 3) - #51
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds polling for dashboard metrics, VPN, and integrations; introduces command search, Seerr request linking, alert notifications, view switching, and an integration setup view; and documents header behavior and alert suppression states. ChangesDashboard experience
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant deriveAlerts
participant Header
participant Notifications
App->>deriveAlerts: Pass health and integrations
deriveAlerts-->>App: Return ordered alerts
App->>Header: Pass services, groups, and alerts
Header->>Notifications: Render alert controls
Notifications-->>App: Request setup view
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
5f4d2ed to
27c716b
Compare
Phase 3 of the dashboard from #48 — the header controls that phase 1 deliberately left out because the data behind them hadn't landed yet. Search filters the service catalog and opens what you pick, focused with "/" or Ctrl/Cmd+K. Only services with a published port are offered as results, since opening one is all a result does; matches without a web UI are named in a footer line rather than becoming rows that do nothing on Enter. Request deep-links to Seerr instead of posting. The dashboard is read-only and ships no auth, so a request endpoint here would let anyone who can reach the page add to the library under Seerr's credentials with no record of who did it. Alerts are derived in the browser from /api/health plus /api/integrations — both already polled — so no new endpoint re-fetches what the client can assemble for free. The integrations poll moves up to App so one request feeds both the bell and Setup. Two states deliberately stay quiet: `absent` services, so trimming compose doesn't produce permanent alerts, and `waiting` integrations, which are normal on a clean install and would otherwise open a first boot with an inbox that clears itself. An unreachable socket proxy makes every service read `absent`, so that case short-circuits to one alert naming the proxy rather than reporting nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
27c716b to
87c9a6f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
dashboard/server/src/discovery.ts (1)
127-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo unit coverage for
discoverArr/discoverTautulli/discoverSeerr/discoverOne.
discovery.test.tsonly exercisesxmlTag/iniValuevia__test. The higher-level per-source discovery functions (waiting/blocked/live transitions, env-override precedence) are untested, even thoughDISCOVER_ROOTis already overridable via env, making them straightforward to test against a temp fixture directory.🤖 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/server/src/discovery.ts` around lines 127 - 227, Add unit coverage for discoverArr, discoverTautulli, discoverSeerr, and discoverOne using a temporary DISCOVER_ROOT fixture directory. Test missing and incomplete configuration waiting states, Tautulli’s blocked state when the API is disabled, valid live discovery for each source, Seerr parse-retry behavior, and discoverOne’s environment-variable override taking precedence over discovered credentials.dashboard/web/src/components/Gauges.tsx (1)
20-57: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGauges and VpnCard don't propagate polling failures or upstream reasons, unlike CommandCenter's PanelBody-driven panels. Both widgets were built without the
error-prop/PanelEmptypattern thatPanelBody(Panel.tsxlines 101-114) already establishes for every other panel in this PR, so a persistent transport failure or an upstream-declinedreason/hintis silently lost on the resource strip and VPN card.
dashboard/web/src/components/Gauges.tsx#L20-L57: add anerror?: string | nullprop and renderPanelEmpty(reason "Dashboard API unreachable" +erroras hint) on the first card whendatais null anderroris set, mirroringPanelBody.dashboard/web/src/app/App.tsx#L78-L140: forwardvpn.errorinto<Sidebar>andmetrics.errorinto<Gauges>so the new prop above has data to work with.dashboard/web/src/app/Sidebar.tsx#L160-L214: inVpnCard, surfacevpn.reason/vpn.hintwhenvpn.available === falseinstead of collapsing that case into the generic "RPC unreachable" text, and account for a transport-levelvpn.errordistinctly from an upstream decline.🤖 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/web/src/components/Gauges.tsx` around lines 20 - 57, Update Gauges to accept an optional error prop and render PanelEmpty on the first placeholder when data is null and error is present, using “Dashboard API unreachable” as the reason and error as the hint. In dashboard/web/src/app/App.tsx lines 78-140, forward vpn.error to Sidebar and metrics.error to Gauges. In dashboard/web/src/app/Sidebar.tsx lines 160-214, update VpnCard to display vpn.reason and vpn.hint for unavailable upstream responses, while handling transport-level vpn.error separately from an upstream decline.dashboard/web/src/styles/autoplexx.css (1)
110-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse design tokens for
.ap-badgespacing/radius, not hard-coded pixel values.Colors correctly use
var(--ap-amber)/var(--color-bg), but the offsets, size, radius, and font-size (-4px,16px,8px,10px) are all hard-coded. As per coding guidelines, "Use thevar(--*)design tokens fromweb/src/styles/nocturne.cssfor colors, spacing, radii, and shadows instead of hard-coded values."♻️ Example using nearest tokens (adjust to whatever nocturne.css actually exposes)
.ap-badge { position: absolute; - top: -4px; - right: -4px; - min-width: 16px; - height: 16px; - padding: 0 4px; - border-radius: 8px; + top: calc(-1 * var(--space-1)); + right: calc(-1 * var(--space-1)); + min-width: var(--space-4); + height: var(--space-4); + padding: 0 var(--space-1); + border-radius: var(--radius-full, 8px); background: var(--ap-amber); color: var(--color-bg); - font-size: 10px; font-weight: 600; - line-height: 16px; + line-height: var(--space-4); text-align: center; }Separately, worth confirming badge text contrast in the light theme, where
--ap-amberbecomes a mid-lightnessoklch(62% 0.15 78)and--color-bgbecomes a near-white#f3f5fe— the dark-theme pairing (light amber + presumably dark--color-bg) likely has more headroom than this one.🤖 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/web/src/styles/autoplexx.css` around lines 110 - 125, Update the `.ap-badge` styles to replace hard-coded offsets, dimensions, padding, radius, line-height, and font-size with the nearest available `var(--*)` design tokens from `nocturne.css`. Preserve the existing layout, badge appearance, and color variables, and verify the selected tokens cover all spacing and sizing values called out in the review.Source: Coding guidelines
🤖 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/README.md`:
- Around line 73-86: Make the file path reference in the widget instructions
consistent by updating the `hintFor()` example to include the same `server/src/`
prefix used for the source-module path, without changing the surrounding
guidance.
In `@dashboard/server/src/http.ts`:
- Around line 56-69: Update the memoize() cache-refresh logic so a loader result
replaces the cached value and expiresAt only when the existing cached result is
available; retain the last successful result when a refresh returns Unavailable,
while preserving normal caching behavior for successful loads.
In `@dashboard/server/src/sources/prometheus.ts`:
- Around line 28-36: Update the Prometheus query helper query to catch getJson
failures and return null, preserving its existing null behavior for unsuccessful
responses or missing/invalid samples so one failed parallel query does not
reject load's Promise.all.
In `@dashboard/server/src/sources/seerr.ts`:
- Around line 163-169: Update getRequests in
dashboard/server/src/sources/seerr.ts (lines 163-169), the corresponding
Tautulli flow in dashboard/server/src/sources/tautulli.ts (lines 149-158), and
the Sonarr flow in dashboard/server/src/sources/upcoming.ts (lines 41-47) to add
and use a hintFor(reason) helper, mirroring transmission.ts. Pass the generated
hint to safely(load, hint) or post-process the unavailable result so
authentication rejection and connection failures receive distinct, actionable
advice.
---
Nitpick comments:
In `@dashboard/server/src/discovery.ts`:
- Around line 127-227: Add unit coverage for discoverArr, discoverTautulli,
discoverSeerr, and discoverOne using a temporary DISCOVER_ROOT fixture
directory. Test missing and incomplete configuration waiting states, Tautulli’s
blocked state when the API is disabled, valid live discovery for each source,
Seerr parse-retry behavior, and discoverOne’s environment-variable override
taking precedence over discovered credentials.
In `@dashboard/web/src/components/Gauges.tsx`:
- Around line 20-57: Update Gauges to accept an optional error prop and render
PanelEmpty on the first placeholder when data is null and error is present,
using “Dashboard API unreachable” as the reason and error as the hint. In
dashboard/web/src/app/App.tsx lines 78-140, forward vpn.error to Sidebar and
metrics.error to Gauges. In dashboard/web/src/app/Sidebar.tsx lines 160-214,
update VpnCard to display vpn.reason and vpn.hint for unavailable upstream
responses, while handling transport-level vpn.error separately from an upstream
decline.
In `@dashboard/web/src/styles/autoplexx.css`:
- Around line 110-125: Update the `.ap-badge` styles to replace hard-coded
offsets, dimensions, padding, radius, line-height, and font-size with the
nearest available `var(--*)` design tokens from `nocturne.css`. Preserve the
existing layout, badge appearance, and color variables, and verify the selected
tokens cover all spacing and sizing values called out in the review.
🪄 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 Plus
Run ID: 104678cb-6163-453e-b0b4-d9f7177e6e04
📒 Files selected for processing (29)
CLAUDE.mdREADME.mddashboard/README.mddashboard/server/src/config.tsdashboard/server/src/discovery.test.tsdashboard/server/src/discovery.tsdashboard/server/src/http.tsdashboard/server/src/index.tsdashboard/server/src/sources/activity.tsdashboard/server/src/sources/arr.tsdashboard/server/src/sources/prometheus.tsdashboard/server/src/sources/seerr.tsdashboard/server/src/sources/sources.test.tsdashboard/server/src/sources/tautulli.tsdashboard/server/src/sources/transmission.tsdashboard/server/src/sources/upcoming.tsdashboard/web/src/alerts.tsdashboard/web/src/app/App.tsxdashboard/web/src/app/Header.tsxdashboard/web/src/app/Sidebar.tsxdashboard/web/src/components/CommandSearch.tsxdashboard/web/src/components/Gauges.tsxdashboard/web/src/components/Notifications.tsxdashboard/web/src/components/Panel.tsxdashboard/web/src/hooks/useDismissable.tsdashboard/web/src/styles/autoplexx.cssdashboard/web/src/types.tsdashboard/web/src/views/CommandCenter.tsxdashboard/web/src/views/Setup.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (3)
dashboard/server/src/discovery.ts (1)
127-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo unit coverage for
discoverArr/discoverTautulli/discoverSeerr/discoverOne.
discovery.test.tsonly exercisesxmlTag/iniValuevia__test. The higher-level per-source discovery functions (waiting/blocked/live transitions, env-override precedence) are untested, even thoughDISCOVER_ROOTis already overridable via env, making them straightforward to test against a temp fixture directory.🤖 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/server/src/discovery.ts` around lines 127 - 227, Add unit coverage for discoverArr, discoverTautulli, discoverSeerr, and discoverOne using a temporary DISCOVER_ROOT fixture directory. Test missing and incomplete configuration waiting states, Tautulli’s blocked state when the API is disabled, valid live discovery for each source, Seerr parse-retry behavior, and discoverOne’s environment-variable override taking precedence over discovered credentials.dashboard/web/src/components/Gauges.tsx (1)
20-57: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGauges and VpnCard don't propagate polling failures or upstream reasons, unlike CommandCenter's PanelBody-driven panels. Both widgets were built without the
error-prop/PanelEmptypattern thatPanelBody(Panel.tsxlines 101-114) already establishes for every other panel in this PR, so a persistent transport failure or an upstream-declinedreason/hintis silently lost on the resource strip and VPN card.
dashboard/web/src/components/Gauges.tsx#L20-L57: add anerror?: string | nullprop and renderPanelEmpty(reason "Dashboard API unreachable" +erroras hint) on the first card whendatais null anderroris set, mirroringPanelBody.dashboard/web/src/app/App.tsx#L78-L140: forwardvpn.errorinto<Sidebar>andmetrics.errorinto<Gauges>so the new prop above has data to work with.dashboard/web/src/app/Sidebar.tsx#L160-L214: inVpnCard, surfacevpn.reason/vpn.hintwhenvpn.available === falseinstead of collapsing that case into the generic "RPC unreachable" text, and account for a transport-levelvpn.errordistinctly from an upstream decline.🤖 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/web/src/components/Gauges.tsx` around lines 20 - 57, Update Gauges to accept an optional error prop and render PanelEmpty on the first placeholder when data is null and error is present, using “Dashboard API unreachable” as the reason and error as the hint. In dashboard/web/src/app/App.tsx lines 78-140, forward vpn.error to Sidebar and metrics.error to Gauges. In dashboard/web/src/app/Sidebar.tsx lines 160-214, update VpnCard to display vpn.reason and vpn.hint for unavailable upstream responses, while handling transport-level vpn.error separately from an upstream decline.dashboard/web/src/styles/autoplexx.css (1)
110-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse design tokens for
.ap-badgespacing/radius, not hard-coded pixel values.Colors correctly use
var(--ap-amber)/var(--color-bg), but the offsets, size, radius, and font-size (-4px,16px,8px,10px) are all hard-coded. As per coding guidelines, "Use thevar(--*)design tokens fromweb/src/styles/nocturne.cssfor colors, spacing, radii, and shadows instead of hard-coded values."♻️ Example using nearest tokens (adjust to whatever nocturne.css actually exposes)
.ap-badge { position: absolute; - top: -4px; - right: -4px; - min-width: 16px; - height: 16px; - padding: 0 4px; - border-radius: 8px; + top: calc(-1 * var(--space-1)); + right: calc(-1 * var(--space-1)); + min-width: var(--space-4); + height: var(--space-4); + padding: 0 var(--space-1); + border-radius: var(--radius-full, 8px); background: var(--ap-amber); color: var(--color-bg); - font-size: 10px; font-weight: 600; - line-height: 16px; + line-height: var(--space-4); text-align: center; }Separately, worth confirming badge text contrast in the light theme, where
--ap-amberbecomes a mid-lightnessoklch(62% 0.15 78)and--color-bgbecomes a near-white#f3f5fe— the dark-theme pairing (light amber + presumably dark--color-bg) likely has more headroom than this one.🤖 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/web/src/styles/autoplexx.css` around lines 110 - 125, Update the `.ap-badge` styles to replace hard-coded offsets, dimensions, padding, radius, line-height, and font-size with the nearest available `var(--*)` design tokens from `nocturne.css`. Preserve the existing layout, badge appearance, and color variables, and verify the selected tokens cover all spacing and sizing values called out in the review.Source: Coding guidelines
🤖 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/README.md`:
- Around line 73-86: Make the file path reference in the widget instructions
consistent by updating the `hintFor()` example to include the same `server/src/`
prefix used for the source-module path, without changing the surrounding
guidance.
In `@dashboard/server/src/http.ts`:
- Around line 56-69: Update the memoize() cache-refresh logic so a loader result
replaces the cached value and expiresAt only when the existing cached result is
available; retain the last successful result when a refresh returns Unavailable,
while preserving normal caching behavior for successful loads.
In `@dashboard/server/src/sources/prometheus.ts`:
- Around line 28-36: Update the Prometheus query helper query to catch getJson
failures and return null, preserving its existing null behavior for unsuccessful
responses or missing/invalid samples so one failed parallel query does not
reject load's Promise.all.
In `@dashboard/server/src/sources/seerr.ts`:
- Around line 163-169: Update getRequests in
dashboard/server/src/sources/seerr.ts (lines 163-169), the corresponding
Tautulli flow in dashboard/server/src/sources/tautulli.ts (lines 149-158), and
the Sonarr flow in dashboard/server/src/sources/upcoming.ts (lines 41-47) to add
and use a hintFor(reason) helper, mirroring transmission.ts. Pass the generated
hint to safely(load, hint) or post-process the unavailable result so
authentication rejection and connection failures receive distinct, actionable
advice.
---
Nitpick comments:
In `@dashboard/server/src/discovery.ts`:
- Around line 127-227: Add unit coverage for discoverArr, discoverTautulli,
discoverSeerr, and discoverOne using a temporary DISCOVER_ROOT fixture
directory. Test missing and incomplete configuration waiting states, Tautulli’s
blocked state when the API is disabled, valid live discovery for each source,
Seerr parse-retry behavior, and discoverOne’s environment-variable override
taking precedence over discovered credentials.
In `@dashboard/web/src/components/Gauges.tsx`:
- Around line 20-57: Update Gauges to accept an optional error prop and render
PanelEmpty on the first placeholder when data is null and error is present,
using “Dashboard API unreachable” as the reason and error as the hint. In
dashboard/web/src/app/App.tsx lines 78-140, forward vpn.error to Sidebar and
metrics.error to Gauges. In dashboard/web/src/app/Sidebar.tsx lines 160-214,
update VpnCard to display vpn.reason and vpn.hint for unavailable upstream
responses, while handling transport-level vpn.error separately from an upstream
decline.
In `@dashboard/web/src/styles/autoplexx.css`:
- Around line 110-125: Update the `.ap-badge` styles to replace hard-coded
offsets, dimensions, padding, radius, line-height, and font-size with the
nearest available `var(--*)` design tokens from `nocturne.css`. Preserve the
existing layout, badge appearance, and color variables, and verify the selected
tokens cover all spacing and sizing values called out in the review.
🪄 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 Plus
Run ID: 104678cb-6163-453e-b0b4-d9f7177e6e04
📒 Files selected for processing (29)
CLAUDE.mdREADME.mddashboard/README.mddashboard/server/src/config.tsdashboard/server/src/discovery.test.tsdashboard/server/src/discovery.tsdashboard/server/src/http.tsdashboard/server/src/index.tsdashboard/server/src/sources/activity.tsdashboard/server/src/sources/arr.tsdashboard/server/src/sources/prometheus.tsdashboard/server/src/sources/seerr.tsdashboard/server/src/sources/sources.test.tsdashboard/server/src/sources/tautulli.tsdashboard/server/src/sources/transmission.tsdashboard/server/src/sources/upcoming.tsdashboard/web/src/alerts.tsdashboard/web/src/app/App.tsxdashboard/web/src/app/Header.tsxdashboard/web/src/app/Sidebar.tsxdashboard/web/src/components/CommandSearch.tsxdashboard/web/src/components/Gauges.tsxdashboard/web/src/components/Notifications.tsxdashboard/web/src/components/Panel.tsxdashboard/web/src/hooks/useDismissable.tsdashboard/web/src/styles/autoplexx.cssdashboard/web/src/types.tsdashboard/web/src/views/CommandCenter.tsxdashboard/web/src/views/Setup.tsx
🛑 Comments failed to post (4)
dashboard/README.md (1)
73-86: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Inconsistent file path reference.
Line 75 uses the full path
server/src/sources/, but line 83 shortens the same reference tosources/transmission.ts, dropping theserver/src/prefix within the same section.-fixes the problem. See `hintFor()` in `sources/transmission.ts` — an auth +fixes the problem. See `hintFor()` in `server/src/sources/transmission.ts` — an auth📝 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.## Adding a widget 1. Add a source module under `server/src/sources/`. It must export a `memoize`d function returning `Result<T>` — use `safely()` so a failure becomes `{ available: false, reason, hint }` rather than a rejection. 2. Register a route in `server/src/index.ts`. 3. Add the payload type to `web/src/types.ts` and render it with `<PanelBody>`, which handles the loading, unavailable and empty cases for you. The `hint` is the part that matters: it should name the one concrete step that fixes the problem. See `hintFor()` in `server/src/sources/transmission.ts` — an auth failure and an unreachable host need different advice, and a generic hint sends people looking in the wrong place.🧰 Tools
🪛 LanguageTool
[style] ~82-~82: Consider using a different verb for a more formal wording.
Context: ... should name the one concrete step that fixes the problem. SeehintFor()in `source...(FIX_RESOLVE)
🤖 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/README.md` around lines 73 - 86, Make the file path reference in the widget instructions consistent by updating the `hintFor()` example to include the same `server/src/` prefix used for the source-module path, without changing the surrounding guidance.dashboard/server/src/http.ts (1)
56-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect memoize() to see if it retains the previous value on a failed/Unavailable refresh. fd -a cache.ts dashboard/server/src cat -n dashboard/server/src/cache.ts 2>/dev/null ast-grep run --pattern 'function memoize($$$) { $$$ }' --lang typescript dashboard/server/src/cache.ts 2>/dev/nullRepository: joshdev8/AutoPlexx
Length of output: 1250
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Result type definitions:" rg -n "type Result|interface Result|export .*Result|Unavailable|unavailable" dashboard/server/src -g '*.ts' -g '*.tsx' echo echo "Memoized source usages:" rg -n "memoize\\(.*=\\>.*safely|memoize\\(.*load|safely\\(load" dashboard/server/src -g '*.ts' echo echo "cache.ts imports/usages in server source:" rg -n "from ['\"].*cache|memoize\\(" dashboard/server/src -g '*.ts' -g '*.tsx'Repository: joshdev8/AutoPlexx
Length of output: 4016
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "cache.ts (for reference):" cat -n dashboard/server/src/cache.ts 2>/dev/null || true echo echo "http.ts unavailable/result helpers:" sed -n '1,90p' dashboard/server/src/http.ts | cat -n echo echo "activity.ts loader route around memoized function:" sed -n '50,95p' dashboard/server/src/sources/activity.ts | cat -n echo echo "prometheus.ts loader route around memoized function:" sed -n '90,125p' dashboard/server/src/sources/prometheus.ts | cat -n echo echo "seerr.ts loader route around memoized function:" sed -n '150,175p' dashboard/server/src/sources/seerr.ts | cat -n echo echo "tautulli.ts loader route around memoized function:" sed -n '140,165p' dashboard/server/src/sources/tautulli.ts | cat -n echo echo "transmission.ts loader route around memoized function:" sed -n '170,190p' dashboard/server/src/sources/transmission.ts | cat -n echo echo "upcoming.ts loader route around memoized function:" sed -n '35,55p' dashboard/server/src/sources/upcoming.ts | cat -nRepository: joshdev8/AutoPlexx
Length of output: 10098
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from dataclasses import dataclass from typing import Optional TTL_MS = 10_000 `@dataclass` class Unavailable: available: bool reason: str hint: Optional[str] = None class State: def __init__(self): self.value: Optional[object] = None self.expires_at = 0 self.in_flight: Optional[object] = None def memoize(fn: object, ttl_ms: int, state: State): async def wrapper(): if state.value is not None and state.expires_at > (clock := state.time()): return state.value if state.in_flight is not None: return state.values[state.in_flight]() result_future = state.time_future(fn()) state.in_flight = result_future result = await result_future state.value = result state.expires_at = state.time() + ttl_ms state.in_flight = None return result return wrapper class CacheClock(State): def __init__(self): super().__init__() self.now = 0 self.values = {} def time(self): return self.now def advance(self, ms): self.now += ms def time_future(self, future): idx = len(self.values) self.values[idx] = future return idx async def await_value(self, idx): return self.values[idx] async def main(): clock = CacheClock() get = lambda payload: {**payload, "available": True} memo = memoize(get({"items": ["last good item"]}), TTL_MS, clock) # Successful cache populate. v = await memo() assert v == get({"items": ["last good item"]}), "initial failed" clock.advance(TTL_MS // 2) # Miss after expiry -> fetch Unavailable. clock.time_future(Unavailable(False, "fetch failed")) v = await memo() assert v == Unavailable(False, "fetch failed"), "failed fetch failed" # Request during stale failure TTL. clock.advance(TTL_MS // 2) v = await memo() assert v.available is False, "failed TTL should not return last good" assert v == Unavailable(False, "fetch failed"), "stale failed failed" # Successful fetch after TTL. clock.time_future(get({"items": ["new item"]})) v = await memo() assert v == get({"items": ["new item"]}), "new fetch failed" clock.time_future(Unavailable(False, "failed refresh")) v = await memo() assert v.available is True, "failed refresh should not overwrite" assert v == get({"items": ["new item"]}), "overwrite logic failed" PYRepository: joshdev8/AutoPlexx
Length of output: 156
Preserve last-good results on stale TTL hits.
memoize()caches the loader’s successful result, but if the next loader resolves toUnavailable, this value +expiresAtget overwritten. The widget cache should only refresh when the last cached result isavailable, so a failed poll does not blank the widget for the remainder of the TTL.🤖 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/server/src/http.ts` around lines 56 - 69, Update the memoize() cache-refresh logic so a loader result replaces the cached value and expiresAt only when the existing cached result is available; retain the last successful result when a refresh returns Unavailable, while preserving normal caching behavior for successful loads.Source: Coding guidelines
dashboard/server/src/sources/prometheus.ts (1)
28-36: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
One failing PromQL query blanks the whole gauges panel.
query()doesn't catchgetJsonfailures, so if any single one of the 8 parallel queries throws (bad PromQL, transient HTTP error),Promise.allinload()rejects andsafely()marks the entire metrics widget unavailable — even though the other 7 metrics may be healthy.query()already tolerates "soft" failures (non-success status, missing sample) by returningnull; extending that to thrown errors keeps a single flaky metric from taking down gauges that would otherwise render.🛡️ Proposed fix
async function query(expr: string): Promise<number | null> { const url = `${config.upstream.prometheus}/api/v1/query?query=${encodeURIComponent(expr)}`; - const body = await getJson<PromResponse>(url); + const body = await getJson<PromResponse>(url).catch(() => null); + if (!body) return null; if (body.status !== 'success') return null; const raw = body.data?.result?.[0]?.value?.[1]; if (raw === undefined) return null; const parsed = Number(raw); return Number.isFinite(parsed) ? parsed : null; }📝 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.async function query(expr: string): Promise<number | null> { const url = `${config.upstream.prometheus}/api/v1/query?query=${encodeURIComponent(expr)}`; const body = await getJson<PromResponse>(url).catch(() => null); if (!body) return null; if (body.status !== 'success') return null; const raw = body.data?.result?.[0]?.value?.[1]; if (raw === undefined) return null; const parsed = Number(raw); return Number.isFinite(parsed) ? parsed : null; }🤖 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/server/src/sources/prometheus.ts` around lines 28 - 36, Update the Prometheus query helper query to catch getJson failures and return null, preserving its existing null behavior for unsuccessful responses or missing/invalid samples so one failed parallel query does not reject load's Promise.all.dashboard/server/src/sources/seerr.ts (1)
163-169: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing differentiated hints on post-credential-live failures. All three sources call
safely(load)with nohintargument once their credential is already live, so a rejected API key and an unreachable host both surface the same barereason— unliketransmission.ts'shintFor(), which is the pattern the coding guidelines call for ("rejected credentials and unreachable hosts must receive different advice rather than a generic hint").
dashboard/server/src/sources/seerr.ts#L163-L169: add ahintFor(reason)helper (mirroringtransmission.ts) and pass its result intosafely(load, hint)or post-process theUnavailableresult the waytransmission.ts'sgetDownloadsdoes.dashboard/server/src/sources/tautulli.ts#L149-L158: same — differentiate an "authentication rejected" reason from a "connection refused"/"host not found"/"upstream timed out" reason with distinct hints.dashboard/server/src/sources/upcoming.ts#L41-L47: same — Sonarr auth failures and unreachable-host failures should get distinct hints instead of none.As per coding guidelines, "Hints generated by hintFor() must identify the actual fix; rejected credentials and unreachable hosts must receive different advice rather than a generic hint."
📍 Affects 3 files
dashboard/server/src/sources/seerr.ts#L163-L169(this comment)dashboard/server/src/sources/tautulli.ts#L149-L158dashboard/server/src/sources/upcoming.ts#L41-L47🤖 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/server/src/sources/seerr.ts` around lines 163 - 169, Update getRequests in dashboard/server/src/sources/seerr.ts (lines 163-169), the corresponding Tautulli flow in dashboard/server/src/sources/tautulli.ts (lines 149-158), and the Sonarr flow in dashboard/server/src/sources/upcoming.ts (lines 41-47) to add and use a hintFor(reason) helper, mirroring transmission.ts. Pass the generated hint to safely(load, hint) or post-process the unavailable result so authentication rejection and connection failures receive distinct, actionable advice.Source: Coding guidelines
Two findings from the review that fall inside this PR's diff: The alert badge failed contrast in the light theme. It is the one place amber sits behind text rather than serving as a dot, border or bar, and light's --ap-amber gave only 3.4:1 against --color-bg — under the 4.5:1 AA asks for at 10px. Dark theme was already fine at 9.9:1. Rather than move --ap-amber and disturb every tag and gauge that reads it, the badge now takes its own --ap-badge token, which light overrides to oklch(52% 0.15 78) for 5.1:1. The badge's geometry was also four independent magic numbers that had to agree. nocturne's spacing scale is a layout scale — 2.8 / 5.6 / 8.4 / 11.2px — and no step reaches the 16px an icon-corner badge needs, so tokenising the sizes outright would have changed the design. Instead the size is stated once and the offset, radius and line-height derive from it. Computed values are unchanged: 16x16, radius 8px, offset -4px. Also makes the README's hintFor() path carry the same server/src/ prefix the surrounding instructions use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note on the previous review's scope: it ran while this PR's merge-base was stale, so it saw the whole 29-file stack rather than this PR's 9 files. Most of its findings were against #50 code that has since merged to Addressed here (both inside this diff):
Not addressed here — all against files this PR doesn't touch, and all now merged on
On the spacing-token suggestion specifically: nocturne's scale is a layout scale (2.8 / 5.6 / 8.4 / 11.2px) and no step reaches the 16px an icon-corner badge needs, so mapping the sizes onto it as proposed would have shrunk the badge. The size is stated once and the offset/radius/line-height derive from it instead, which addresses the keep-in-sync concern without changing the design. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Phase 3 of the dashboard from #48: the header controls that #49 deliberately shipped as empty space, because they'd have been dead until phase 2's data landed.
Based on
feat/dashboard-widgets(#50) — review that one first; this PR retargets tomainonce it merges.The three controls
/orCtrl/Cmd+Kto focusThe avatar from the design stays out — there's no auth for it to represent.
What deliberately doesn't raise an alert
absentservices. A user who trimmed services out of their compose file would otherwise get a permanent list of alerts for things they chose not to run.waitingintegrations. That's the normal state on a clean install, where a service simply hasn't written its config file yet. Alerting would mean a first boot opens with a full inbox that clears itself.absent, which would render the loudest possible problem as total silence. That case short-circuits to a single alert naming the proxy.Also here
App— the bell and Setup now share one request instead of each running their own.Verification
Driven in a browser against the live stack (18/18 up), not just built:
soreturns Sonarr (name prefix) ahead of cAdvisor and FlareSolverr, witharia-activedescendanttracking the highlightwatchreturns no openable rows and correctly reports "Also matched, no web UI: Watchlistarr" — Watchtower, asystemservice, stays out of search entirelywaiting— confirming waiting is not an alerttypecheck,lint, 40 tests andbuildall cleanNote for review
web/still has no test runner, soalerts.tsand the search ranking inCommandSearch.tsxare verified in the browser rather than by unit test, despite both being pure functions written to be testable. Adding one is web infrastructure rather than a feature, so it belongs with the responsive pass rather than in here — happy to do it either way.Refs #48
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation