fix: increase responsiveness#54
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
📝 WalkthroughWalkthroughAdds a 30s request timeout to the shared API helper and introduces staged, user-facing loading indicators (via a new LoadingState component and log-derived progressDetail) to replace blank loading screens and distinguish initial vs. background refresh loading states. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/app/routes/home.tsx (1)
79-83: 🧹 Nitpick | 🔵 TrivialConsider wrapping
loadMakersinuseCallbackfor safer effect dependencies.The
loadMakersfunction is referenced inuseEffectbut not included in the dependency array. While this works because the function doesn't depend on props/state that change independently, the pattern can become fragile if the function is modified later.♻️ Safer pattern with useCallback
+import { useEffect, useRef, useState, useCallback } from "react"; -import { useEffect, useRef, useState } from "react"; ... - async function loadMakers() { + const loadMakers = useCallback(async () => { const initial = isInitialLoad.current; ... - } + }, []); useEffect(() => { loadMakers(); const interval = setInterval(loadMakers, 15_000); return () => clearInterval(interval); - }, []); + }, [loadMakers]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/routes/home.tsx` around lines 79 - 83, Wrap the loadMakers function in useCallback to stabilize its identity and then include that callback in the useEffect dependency array; specifically change the current loadMakers implementation to a memoized callback (useCallback(() => { ... }, [/* any real deps */])) and update the useEffect to depend on loadMakers so the setInterval uses the stable function and the cleanup/reauthorization remains correct (adjust any internal dependencies of loadMakers as needed).frontend/app/routes/makerDetails/index.tsx (1)
65-73:⚠️ Potential issue | 🟡 MinorLoading detail message is misleading.
The message "Resolving Tor address…" is set but the subsequent
torAddressanddataDirfetches are fire-and-forget (not awaited). The loading state will immediately transition to complete while these calls are still in flight, making the detail message inaccurate.Consider either:
- Awaiting these calls before clearing loading state, or
- Removing this detail message since it doesn't reflect actual loading behavior
🔧 Option 1: Await the calls
if (initial) setLoadingDetail("Resolving Tor address…"); - monitoring - .torAddress(id) - .then(setTorAddress) - .catch(() => {}); - monitoring - .dataDir(id) - .then(setDataDir) - .catch(() => {}); + await Promise.allSettled([ + monitoring.torAddress(id).then(setTorAddress), + monitoring.dataDir(id).then(setDataDir), + ]); } catch (e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/routes/makerDetails/index.tsx` around lines 65 - 73, The loading message "Resolving Tor address…" is misleading because you call monitoring.torAddress(id) and monitoring.dataDir(id) as fire-and-forget promises; update the logic in the maker details loader (where setLoadingDetail, monitoring.torAddress, monitoring.dataDir, and initial are used) to either await both calls (e.g., Promise.all on monitoring.torAddress(id) and monitoring.dataDir(id)) before clearing the loading state so the message reflects real work, or remove the setLoadingDetail("Resolving Tor address…") call entirely if you want to keep the fetches non-blocking; ensure setTorAddress and setDataDir still get invoked with resolved values when choosing the await approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api.ts`:
- Around line 152-160: Remove the redundant clearTimeout(timer) call from the
catch block in frontend/app/api.ts: keep the existing AbortError detection and
rethrow logic (throw new ApiError(...) and throw err) but rely on the finally
block to clear the timer; ensure only the finally block contains
clearTimeout(timer) so the timer is always cleared once.
In `@frontend/app/routes/makersetup.tsx`:
- Around line 163-165: The current update uses latestProgressDetail(next) and
only calls setProgressDetail(detail) when detail is truthy, which leaves
progressDetail stale if no pattern matches; change the logic around
latestProgressDetail()/setProgressDetail so you always call setProgressDetail
with the matcher result (explicitly set to null when no match) — update the code
that uses latestProgressDetail(next) and setProgressDetail to set null when
there is no detail (so PROGRESS_PATTERNS/ stageConfig fallback shows) and ensure
symbols referenced are latestProgressDetail, setProgressDetail, progressDetail,
PROGRESS_PATTERNS and stageConfig.
- Around line 61-90: Hoist the static patterns array out of latestProgressDetail
to module scope (e.g., const LATEST_PROGRESS_PATTERNS: [string,string][] =
[...]) so it is allocated once; then update latestProgressDetail to iterate over
LATEST_PROGRESS_PATTERNS instead of recreating patterns on every call,
preserving the same entries, order and typing and keeping the constant
immutable.
---
Outside diff comments:
In `@frontend/app/routes/home.tsx`:
- Around line 79-83: Wrap the loadMakers function in useCallback to stabilize
its identity and then include that callback in the useEffect dependency array;
specifically change the current loadMakers implementation to a memoized callback
(useCallback(() => { ... }, [/* any real deps */])) and update the useEffect to
depend on loadMakers so the setInterval uses the stable function and the
cleanup/reauthorization remains correct (adjust any internal dependencies of
loadMakers as needed).
In `@frontend/app/routes/makerDetails/index.tsx`:
- Around line 65-73: The loading message "Resolving Tor address…" is misleading
because you call monitoring.torAddress(id) and monitoring.dataDir(id) as
fire-and-forget promises; update the logic in the maker details loader (where
setLoadingDetail, monitoring.torAddress, monitoring.dataDir, and initial are
used) to either await both calls (e.g., Promise.all on monitoring.torAddress(id)
and monitoring.dataDir(id)) before clearing the loading state so the message
reflects real work, or remove the setLoadingDetail("Resolving Tor address…")
call entirely if you want to keep the fetches non-blocking; ensure setTorAddress
and setDataDir still get invoked with resolved values when choosing the await
approach.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 01245abf-4e3e-45da-b383-d47dca83d415
📒 Files selected for processing (6)
frontend/app/api.tsfrontend/app/components/LoadingState.tsxfrontend/app/routes/home.tsxfrontend/app/routes/makerDetails/index.tsxfrontend/app/routes/makerDetails/wallet.tsxfrontend/app/routes/makersetup.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (2)
frontend/app/routes/makersetup.tsx (2)
64-90: 🧹 Nitpick | 🔵 TrivialConsider hoisting the patterns array to module scope.
The
patternsarray is recreated on every call tolatestProgressDetail. Since it's static, moving it outside the function avoids unnecessary allocations during frequent log streaming.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/routes/makersetup.tsx` around lines 64 - 90, Hoist the static patterns array out of the latestProgressDetail function to module scope to avoid reallocating it on every call: create a top-level const (e.g., PATTERNS or patterns) with the same type annotation ([string,string][]) and move the current array literal there, then update latestProgressDetail to reference that top-level symbol instead of recreating the array; ensure you keep the same entries and type so callers and TypeScript types remain unchanged.
170-172:⚠️ Potential issue | 🟡 Minor
progressDetailis never reset tonullonce set.The
if (detail)guard prevents the state from being cleared when no patterns match subsequent log batches. This can result in stale progress messages persisting across stage transitions.🛠️ Suggested fix
// Update progress detail from latest log const detail = latestProgressDetail(next); - if (detail) setProgressDetail(detail); + setProgressDetail(detail);This allows
progressDetailto reset tonullwhen no patterns match, enabling the fallback text instageConfigto display.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/routes/makersetup.tsx` around lines 170 - 172, progressDetail isn't reset because the code only calls setProgressDetail when latestProgressDetail(next) returns a truthy value; change the logic so setProgressDetail is always called with the function result (or explicitly null) to clear stale messages. Specifically, replace the guarded update around latestProgressDetail(next) so that setProgressDetail(detail) (or setProgressDetail(detail ?? null)) is invoked even when detail is falsy, referencing latestProgressDetail and setProgressDetail in makersetup.tsx so the fallback text in stageConfig can display when no pattern matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/app/routes/makersetup.tsx`:
- Around line 64-90: Hoist the static patterns array out of the
latestProgressDetail function to module scope to avoid reallocating it on every
call: create a top-level const (e.g., PATTERNS or patterns) with the same type
annotation ([string,string][]) and move the current array literal there, then
update latestProgressDetail to reference that top-level symbol instead of
recreating the array; ensure you keep the same entries and type so callers and
TypeScript types remain unchanged.
- Around line 170-172: progressDetail isn't reset because the code only calls
setProgressDetail when latestProgressDetail(next) returns a truthy value; change
the logic so setProgressDetail is always called with the function result (or
explicitly null) to clear stale messages. Specifically, replace the guarded
update around latestProgressDetail(next) so that setProgressDetail(detail) (or
setProgressDetail(detail ?? null)) is invoked even when detail is falsy,
referencing latestProgressDetail and setProgressDetail in makersetup.tsx so the
fallback text in stageConfig can display when no pattern matches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2132149f-6d6f-4983-af75-6fc1e691c53b
📒 Files selected for processing (5)
frontend/app/api.tsfrontend/app/components/LoadingState.tsxfrontend/app/routes/home.tsxfrontend/app/routes/makerDetails/index.tsxfrontend/app/routes/makersetup.tsx
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
hulxv
force-pushed
the
fix/increase-responsive
branch
from
6312f16 to
cd01239
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/api.ts`:
- Around line 194-217: The current global REQUEST_TIMEOUT_MS used by request()
causes ambiguous failures for non-idempotent endpoints (e.g., wallet.send(),
makers.start(), bitcoind.start()); change request(path, options) to accept a
per-call timeout (e.g., options.timeoutMs) that overrides REQUEST_TIMEOUT_MS,
use that value to set the AbortController timer, and preserve existing behavior
when not provided; for known long-running or write endpoints update their
callers to pass longer timeouts or implement idempotency tokens / client-side
deduplication or status-polling (e.g., add idempotency header and/or a polling
flow) so retries don’t create duplicate operations.
In `@frontend/app/components/LoadingState.tsx`:
- Around line 17-31: The LoadingState component currently renders the visual
loader and the optional detail string but does not expose these updates to
assistive tech; update the rendered DOM to include an accessible live region by
adding role="status" and aria-live="polite" to the container that wraps the
textual content (e.g., the div that contains the <p> elements or the parent
"content" wrapper used when inline is true), ensuring the detail text updates
are announced by screen readers while preserving the existing structure and
inline behavior.
In `@frontend/app/routes/home.tsx`:
- Around line 56-60: The useRef call must be moved out of the async helper to
component scope: remove the `const isInitialLoad = useRef(true)` from inside the
`loadMakers` function and instead declare `const isInitialLoad = useRef(true)`
once at the component level (immediately after line 54), then inside
`loadMakers` read `const initial = isInitialLoad.current` and later set
`isInitialLoad.current = false` where the code currently updates it (around the
existing line ~140); this preserves the persistent ref across invocations so
`setRefreshing(true)` executes correctly.
In `@frontend/app/routes/makersetup.tsx`:
- Around line 106-114: latestProgressDetail is never used so progressDetail
remains null; call setProgressDetail(latestProgressDetail(...)) when logs are
initialized and whenever logs change. Specifically, after computing or loading
existingLogs (or the initial logs state) invoke
setProgressDetail(latestProgressDetail(existingLogs)), and inside the logs
update/append handler (where you receive next logs) compute const detail =
latestProgressDetail(next) and call setProgressDetail(detail) so progressDetail
is populated whenever logs change; refer to the functions latestProgressDetail
and setProgressDetail and the log variables existingLogs/next in your updates.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 82fa306b-7685-4507-b9fd-59c89f2e4fb0
📒 Files selected for processing (6)
frontend/app/api.tsfrontend/app/components/LoadingState.tsxfrontend/app/routes/home.tsxfrontend/app/routes/makerDetails/index.tsxfrontend/app/routes/makerDetails/wallet.tsxfrontend/app/routes/makersetup.tsx
| const REQUEST_TIMEOUT_MS = 30_000; | ||
|
|
||
| async function request<T>(path: string, options: RequestInit = {}): Promise<T> { | ||
| const res = await fetch(`/api${path}`, { | ||
| headers: { "Content-Type": "application/json", ...options.headers }, | ||
| ...options, | ||
| }); | ||
| const controller = new AbortController(); | ||
| const timer = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); | ||
|
|
||
| let res: Response; | ||
| try { | ||
| res = await fetch(`/api${path}`, { | ||
| headers: { "Content-Type": "application/json", ...options.headers }, | ||
| signal: controller.signal, | ||
| ...options, | ||
| }); | ||
| } catch (err) { | ||
| if (err instanceof DOMException && err.name === "AbortError") { | ||
| throw new ApiError( | ||
| 0, | ||
| `Request timed out after ${REQUEST_TIMEOUT_MS / 1000}s`, | ||
| ); | ||
| } | ||
| throw err; | ||
| } finally { | ||
| clearTimeout(timer); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Shared timeout implementation:\n'
sed -n '194,217p' frontend/app/api.ts
printf '\nWrappers and affected mutating endpoints:\n'
sed -n '250,381p' frontend/app/api.ts | rg -n 'function (post|put|del)|=>\s*(post|put|del)\('Repository: citadel-tech/maker-dasboard
Length of output: 1450
🏁 Script executed:
wc -l frontend/app/api.tsRepository: citadel-tech/maker-dasboard
Length of output: 94
🏁 Script executed:
cat -n frontend/app/api.ts | head -250Repository: citadel-tech/maker-dasboard
Length of output: 7443
🏁 Script executed:
cat -n frontend/app/api.ts | tail -144Repository: citadel-tech/maker-dasboard
Length of output: 6359
🏁 Script executed:
cd frontend && npx prettier --write . && npm run buildRepository: citadel-tech/maker-dasboard
Length of output: 1477
🏁 Script executed:
# Check if there are any retry mechanisms, idempotency keys, or status polling patterns elsewhere in the codebase
rg -i "retry|idempotent|idempotency|polling|debounce" frontend/app --type ts --type tsx -A 2 -B 2Repository: citadel-tech/maker-dasboard
Length of output: 98
🏁 Script executed:
rg -i "retry|idempotent|idempotency|polling|debounce" frontend/app --type ts -A 2 -B 2Repository: citadel-tech/maker-dasboard
Length of output: 1041
🏁 Script executed:
# Check how mutating endpoints are called in components - look for error handling patterns
rg "wallet\.send|wallet\.sync|makers\.start|makers\.stop|makers\.delete|bitcoind\.start|bitcoind\.stop" frontend/app --type ts -B 3 -A 5Repository: citadel-tech/maker-dasboard
Length of output: 6601
A blanket 30s abort makes write requests ambiguous.
request() backs all post/put/del operations, so timeout affects wallet.send(), makers.start(), bitcoind.start(), and other non-idempotent writes. After 30s the browser stops waiting, but the backend may still complete the operation. The error handler shows a timeout message, but there is no status polling or idempotency mechanism to confirm the operation failed. Users can manually retry, creating duplicate sends or state inconsistency.
Make timeouts configurable per endpoint. For long-running or non-idempotent operations, either implement idempotency tokens and client-side deduplication, or use status polling to confirm operation completion before retrying.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api.ts` around lines 194 - 217, The current global
REQUEST_TIMEOUT_MS used by request() causes ambiguous failures for
non-idempotent endpoints (e.g., wallet.send(), makers.start(),
bitcoind.start()); change request(path, options) to accept a per-call timeout
(e.g., options.timeoutMs) that overrides REQUEST_TIMEOUT_MS, use that value to
set the AbortController timer, and preserve existing behavior when not provided;
for known long-running or write endpoints update their callers to pass longer
timeouts or implement idempotency tokens / client-side deduplication or
status-polling (e.g., add idempotency header and/or a polling flow) so retries
don’t create duplicate operations.
| const content = ( | ||
| <div className="flex flex-col items-center gap-3"> | ||
| <Loader2 className="w-6 h-6 text-orange-500 animate-spin" /> | ||
| <div className="text-center"> | ||
| <p className="text-gray-300 text-sm font-medium">{message}</p> | ||
| {detail && ( | ||
| <p className="text-gray-500 text-xs mt-1 animate-pulse">{detail}</p> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
|
|
||
| if (inline) return content; | ||
|
|
||
| return <div className="flex items-center justify-center h-64">{content}</div>; |
There was a problem hiding this comment.
Expose the loader as a live status region.
The staged detail text is a key part of this PR, but it is only visual right now. Without role="status" and aria-live="polite", screen readers will miss these loading-state updates.
♿ Suggested fix
const content = (
- <div className="flex flex-col items-center gap-3">
- <Loader2 className="w-6 h-6 text-orange-500 animate-spin" />
+ <div
+ className="flex flex-col items-center gap-3"
+ role="status"
+ aria-live="polite"
+ aria-atomic="true"
+ >
+ <Loader2
+ className="w-6 h-6 text-orange-500 animate-spin"
+ aria-hidden="true"
+ />
<div className="text-center">
<p className="text-gray-300 text-sm font-medium">{message}</p>
{detail && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/components/LoadingState.tsx` around lines 17 - 31, The
LoadingState component currently renders the visual loader and the optional
detail string but does not expose these updates to assistive tech; update the
rendered DOM to include an accessible live region by adding role="status" and
aria-live="polite" to the container that wraps the textual content (e.g., the
div that contains the <p> elements or the parent "content" wrapper used when
inline is true), ensuring the detail text updates are announced by screen
readers while preserving the existing structure and inline behavior.
| async function loadMakers(forceSwapRefresh = false) { | ||
| const isInitialLoad = useRef(true); | ||
|
|
||
| const initial = isInitialLoad.current; | ||
| if (!initial) setRefreshing(true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '56,60p' frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 249
🏁 Script executed:
#!/bin/bash
# Get the full structure of the component to understand context
head -80 frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 2639
🏁 Script executed:
#!/bin/bash
# Search for where loadMakers is called
rg "loadMakers" frontend/app/routes/home.tsx -n -A 2 -B 2Repository: citadel-tech/maker-dasboard
Length of output: 659
🏁 Script executed:
#!/bin/bash
# Verify the suggested fix location by checking the component state declarations
sed -n '42,56p' frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 777
🏁 Script executed:
#!/bin/bash
# Check if there are any other hook-related issues in the file
# Look for useEffect, useState, useRef, useCallback, useMemo, useContext, useReducer
rg "use[A-Z]\w+\(" frontend/app/routes/home.tsx -n | head -30Repository: citadel-tech/maker-dasboard
Length of output: 288
🏁 Script executed:
#!/bin/bash
# Verify the exact behavior by checking what happens with isInitialLoad
# Look at how isInitialLoad is used in loadMakers
sed -n '56,75p' frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 828
🏁 Script executed:
#!/bin/bash
# Check if isInitialLoad is referenced anywhere else in the file
rg "isInitialLoad" frontend/app/routes/home.tsx -nRepository: citadel-tech/maker-dasboard
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Check context around where isInitialLoad is set to false (line 140)
sed -n '135,145p' frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 327
🏁 Script executed:
#!/bin/bash
# Let's verify the complete context of the useEffect and understand the lifecycle
sed -n '144,160p' frontend/app/routes/home.tsxRepository: citadel-tech/maker-dasboard
Length of output: 607
🏁 Script executed:
#!/bin/bash
# Final verification: ensure the entire loadMakers function is properly scoped
# and confirm the hook violation by checking ESLint or similar would flag this
wc -l frontend/app/routes/home.tsx && echo "---" && sed -n '52,142p' frontend/app/routes/home.tsx | head -50Repository: citadel-tech/maker-dasboard
Length of output: 2104
Move useRef(true) to component scope.
Line 57 violates React's Rules of Hooks: useRef is called inside the async loadMakers helper function instead of at component level. This breaks the persistent state—each call recreates the ref with true, so isInitialLoad.current = false at line 140 is lost on the next invocation, and the setRefreshing(true) path never executes correctly.
Declare the ref once at component scope (after line 54) and read/update it inside loadMakers.
🔧 Suggested fix
export default function Home() {
const [makerRows, setMakerRows] = useState<MakerRow[]>([]);
const [loading, setLoading] = useState(true);
const [refreshing, setRefreshing] = useState(false);
const [loadingDetail, setLoadingDetail] = useState<string>("");
const [error, setError] = useState<string | null>(null);
const [pending, setPending] = useState<Set<string>>(new Set());
const swapHistoryCache = useRef<Record<string, UtxoInfo[]>>({});
const swapReportCache = useRef<Record<string, SwapReportDto[]>>({});
const lastSwapRefreshAt = useRef(0);
+ const isInitialLoad = useRef(true);
async function loadMakers(forceSwapRefresh = false) {
- const isInitialLoad = useRef(true);
-
const initial = isInitialLoad.current;🧰 Tools
🪛 GitHub Actions: Lint
[error] 57-57: ESLint (react-hooks/rules-of-hooks): React Hook "useRef" is called in function "loadMakers" that is neither a React function component nor a custom React Hook. Function/component names must start uppercase; Hook names must start with "use".
🪛 GitHub Check: frontend
[failure] 57-57:
React Hook "useRef" is called in function "loadMakers" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/routes/home.tsx` around lines 56 - 60, The useRef call must be
moved out of the async helper to component scope: remove the `const
isInitialLoad = useRef(true)` from inside the `loadMakers` function and instead
declare `const isInitialLoad = useRef(true)` once at the component level
(immediately after line 54), then inside `loadMakers` read `const initial =
isInitialLoad.current` and later set `isInitialLoad.current = false` where the
code currently updates it (around the existing line ~140); this preserves the
persistent ref across invocations so `setRefreshing(true)` executes correctly.
| /** Extract the latest meaningful progress detail from logs for user display */ | ||
| function latestProgressDetail(logs: string[]): string | null { | ||
| for (const line of [...logs].reverse()) { | ||
| for (const [needle, detail] of progressPatterns) { | ||
| if (line.includes(needle)) return detail; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
progressDetail is never populated.
latestProgressDetail() and setProgressDetail() are both introduced here, but nothing calls them in this file. That means progressDetail stays null, so all of the new progressDetail ?? ... branches keep rendering the fallback copy and the log-derived status text never appears.
💡 Minimal wiring
setProgressDetail(latestProgressDetail(existingLogs));
...
const detail = latestProgressDetail(next);
setProgressDetail(detail);Also applies to: 142-142
🧰 Tools
🪛 GitHub Check: frontend
[failure] 107-107:
'latestProgressDetail' is defined but never used
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/routes/makersetup.tsx` around lines 106 - 114,
latestProgressDetail is never used so progressDetail remains null; call
setProgressDetail(latestProgressDetail(...)) when logs are initialized and
whenever logs change. Specifically, after computing or loading existingLogs (or
the initial logs state) invoke
setProgressDetail(latestProgressDetail(existingLogs)), and inside the logs
update/append handler (where you receive next logs) compute const detail =
latestProgressDetail(next) and call setProgressDetail(detail) so progressDetail
is populated whenever logs change; refer to the functions latestProgressDetail
and setProgressDetail and the log variables existingLogs/next in your updates.
2 participants
Close #37
Summary by CodeRabbit
New Features
Bug Fixes