refactor: extract relativeTime into shared agentworld util#4428
refactor: extract relativeTime into shared agentworld util#4428bastitva0-blip wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughExtracts a shared ChangesAgent World relativeTime refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/agentworld/pages/FeedSection.tsx (1)
429-566: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLike button has no in-flight guard, unlike Follow.
handleToggleLike(Lines 777-808) computeswillLikeoff of the current optimisticlikeState, but the Like button at Lines 533-542 has nodisabledattribute tied to an in-flight flag — unlike the Follow button, which is disabled viafollowLoadingduring its request (Line 490). A rapid double-click fireslikePostthenunlikePost(or vice-versa) concurrently; whichever response lands last silently overwrites the reconciled state, leaving the UI showing the wrong liked/count until the next refetch.🔧 Proposed fix: add a like-in-flight guard
+ const [likeLoading, setLikeLoading] = useState<Record<string, boolean>>({}); + const handleToggleLike = async (post: GqlPost) => { + if (likeLoading[post.postId]) return; + setLikeLoading((prev) => ({ ...prev, [post.postId]: true })); const current = likeState[post.postId] ?? { liked: post.viewerHasLiked, count: post.likeCount, }; const willLike = !current.liked; ... } catch (err) { setLikeState((prev) => ({ ...prev, [post.postId]: current })); console.error("[FeedSection] like/unlike failed:", err); + } finally { + setLikeLoading((prev) => ({ ...prev, [post.postId]: false })); } };{myAgentId ? ( <button type="button" onClick={() => onToggleLike(post)} + disabled={likeLoading[post.postId] ?? false} className={`flex items-center gap-1 ${Also applies to: 533-561
🤖 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 `@app/src/agentworld/pages/FeedSection.tsx` around lines 429 - 566, The Like action in PostCard can be triggered repeatedly while a request is still in flight, unlike the Follow button which uses followLoading for protection. Add a like-loading/in-flight guard in the same area that owns handleToggleLike and likeState, then pass it into PostCard and disable the like button while a toggle request is pending. Make sure the PostCard like button uses that flag for its disabled state and styling so rapid clicks cannot issue overlapping likePost/unlikePost requests and overwrite the optimistic state.app/src/agentworld/pages/JobsSection.tsx (2)
156-193: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ClientAvataronError fallback targets the wrong DOM node.The
<img>branch (Lines 170-186) and the initials fallback<div>(Lines 188-193) are mutually-exclusiveif/elsereturns — they are never siblings in the rendered DOM. On image load failure,target.nextElementSibling(Line 180) will not be the initials fallback; it will be whatever element actually follows<ClientAvatar />in the parent (e.g. the job title/content<div>inJobRow). This hides the broken image but forces an unrelated sibling'sdisplaytoflex, corrupting that element's layout instead of showing the intended fallback.🐛 Proposed fix using component state instead of manual DOM traversal
function ClientAvatar({ avatarUrl, displayName, }: { avatarUrl?: string; displayName: string; }) { + const [imgFailed, setImgFailed] = useState(false); const initials = displayName .split(" ") .map((w) => w[0] ?? "") .slice(0, 2) .join("") .toUpperCase(); - if (avatarUrl) { + if (avatarUrl && !imgFailed) { return ( <img src={avatarUrl} alt={displayName} className="h-7 w-7 shrink-0 rounded-full object-cover" - onError={(e) => { - // Swap to initials circle on load failure - const target = e.currentTarget as HTMLImageElement; - target.style.display = "none"; - if (target.nextElementSibling) { - (target.nextElementSibling as HTMLElement).style.display = "flex"; - } - }} + onError={() => setImgFailed(true)} /> ); } return ( <div className="flex h-7 w-7 shrink-0 items-center justify-center rounded-full bg-primary-100 text-xs font-medium text-primary-700 dark:bg-primary-900/30 dark:text-primary-400"> {initials || "?"} </div> ); }Requires adding
useStateto the existing React import.🤖 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 `@app/src/agentworld/pages/JobsSection.tsx` around lines 156 - 193, The ClientAvatar fallback logic is using DOM sibling traversal in onError, but the initials fallback is not a sibling in the rendered tree, so it hides the wrong element. Update ClientAvatar to use component state for image load failure instead of nextElementSibling, and render the initials fallback based on that state; also add useState to the React import. Keep the fix localized to ClientAvatar in JobsSection and preserve the existing avatarUrl/displayName behavior.
1135-1167: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDuplicate fetch logic with no request sequencing — stale response can overwrite fresher state.
refetchJobs(Lines 1135-1146) re-implements the same GraphQL fetch as the mountuseEffect(Lines 1148-1167), but only the effect guards against stale results viacancelled. Since the "Post a Job" button (and itsonCreated={refetchJobs}callback) is enabled independently ofjobsState(it doesn't wait for the initial load to finish), a fastrefetchJobs()call can resolve before the slower initial mount fetch, and the initial fetch's stale response will then overwrite the fresher job list when it finally resolves.🔧 Proposed fix: single fetch function with request-id guard, reused by the mount effect
+ const requestIdRef = useRef(0); const refetchJobs = useCallback(() => { + const requestId = ++requestIdRef.current; setJobsState({ status: "loading" }); void apiClient.graphql .jobs({ limit: 50 }) .then((result) => { + if (requestId !== requestIdRef.current) return; const jobs = Array.isArray(result?.jobs) ? result.jobs : []; setJobsState({ status: "ok", jobs }); }) .catch((err: unknown) => { + if (requestId !== requestIdRef.current) return; setJobsState({ status: "error", message: String(err) }); }); }, []); useEffect(() => { - let cancelled = false; - setJobsState({ status: "loading" }); - - void apiClient.graphql - .jobs({ limit: 50 }) - .then((result) => { - if (cancelled) return; - const jobs = Array.isArray(result?.jobs) ? result.jobs : []; - setJobsState({ status: "ok", jobs }); - }) - .catch((err: unknown) => { - if (cancelled) return; - setJobsState({ status: "error", message: String(err) }); - }); - - return () => { - cancelled = true; - }; - }, []); + refetchJobs(); + }, [refetchJobs]);🤖 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 `@app/src/agentworld/pages/JobsSection.tsx` around lines 1135 - 1167, The JobsSection fetch logic is duplicated between refetchJobs and the mount useEffect, and the current cancellation guard only protects the effect, so a slower initial request can overwrite a newer refetchJobs result. Refactor the shared GraphQL jobs fetch into a single helper used by both refetchJobs and the useEffect, and add request sequencing (for example a request id or token) so only the latest response updates setJobsState. Keep the stale-response guard in the JobsSection component near refetchJobs/useEffect.
🧹 Nitpick comments (2)
app/src/agentworld/pages/LedgerSection.tsx (1)
72-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
StatusBlockis duplicated across pages — same candidate for extraction asrelativeTime.This
StatusBlockimplementation is identical to the one inFeedSection.tsx(lines 72-87 per the graph context) and is also referenced byJobsSection.tsx. Given this PR's goal of removing copy-pasted helpers (as done forrelativeTime), consider extractingStatusBlockinto a shared component too, to avoid the same drift risk the issue called out.🤖 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 `@app/src/agentworld/pages/LedgerSection.tsx` around lines 72 - 87, The StatusBlock helper is duplicated across multiple page components, so extract it into a shared reusable component like the existing relativeTime helper. Move the shared rendering logic from StatusBlock into one common location, then update LedgerSection, FeedSection, and JobsSection to import and use that shared component while keeping the same props shape (tone, title, body).app/src/agentworld/pages/JobsSection.tsx (1)
47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
formatAmountandStatusBlockare duplicated verbatim inLedgerSection.tsx.This is the same copy-paste pattern this PR is fixing for
relativeTime(per the linked issue). Consider extractingformatAmount(Lines 47-55) andStatusBlock(Lines 84-99) toapp/src/agentworld/utils/alongsiderelativeTimeto prevent divergence.🤖 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 `@app/src/agentworld/pages/JobsSection.tsx` around lines 47 - 55, The same copy-pasted helpers are still duplicated in JobsSection, so extract both formatAmount and StatusBlock into a shared utility under app/src/agentworld/utils/ and update JobsSection to import them instead of defining them inline. Reuse the existing relativeTime utility pattern as the model, and make sure the shared exports are then used by LedgerSection and JobsSection so the two files no longer diverge.
🤖 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 `@app/src/agentworld/pages/LedgerSection.tsx`:
- Around line 312-324: The metadata list in LedgerSection’s
Object.entries(tx.metadata).map() uses a shorthand Fragment without a key, so
React still treats each iteration as unkeyed. Update the map callback to use an
explicit Fragment with the key on the fragment itself, and keep the existing
dt/dd rendering inside it. Use the LedgerSection map over tx.metadata as the
place to fix this so the list satisfies React’s key requirement.
In `@app/src/agentworld/utils/relativeTime.ts`:
- Around line 1-10: The relativeTime helper can produce "NaNd ago" when iso is
invalid because new Date(iso).getTime() becomes NaN; update relativeTime to
validate the parsed date before calculating mins/hrs/days and return a safe
fallback for malformed input. Use the relativeTime function as the entry point,
add an invalid-date guard right after parsing, and keep the existing formatting
logic unchanged for valid timestamps.
---
Outside diff comments:
In `@app/src/agentworld/pages/FeedSection.tsx`:
- Around line 429-566: The Like action in PostCard can be triggered repeatedly
while a request is still in flight, unlike the Follow button which uses
followLoading for protection. Add a like-loading/in-flight guard in the same
area that owns handleToggleLike and likeState, then pass it into PostCard and
disable the like button while a toggle request is pending. Make sure the
PostCard like button uses that flag for its disabled state and styling so rapid
clicks cannot issue overlapping likePost/unlikePost requests and overwrite the
optimistic state.
In `@app/src/agentworld/pages/JobsSection.tsx`:
- Around line 156-193: The ClientAvatar fallback logic is using DOM sibling
traversal in onError, but the initials fallback is not a sibling in the rendered
tree, so it hides the wrong element. Update ClientAvatar to use component state
for image load failure instead of nextElementSibling, and render the initials
fallback based on that state; also add useState to the React import. Keep the
fix localized to ClientAvatar in JobsSection and preserve the existing
avatarUrl/displayName behavior.
- Around line 1135-1167: The JobsSection fetch logic is duplicated between
refetchJobs and the mount useEffect, and the current cancellation guard only
protects the effect, so a slower initial request can overwrite a newer
refetchJobs result. Refactor the shared GraphQL jobs fetch into a single helper
used by both refetchJobs and the useEffect, and add request sequencing (for
example a request id or token) so only the latest response updates setJobsState.
Keep the stale-response guard in the JobsSection component near
refetchJobs/useEffect.
---
Nitpick comments:
In `@app/src/agentworld/pages/JobsSection.tsx`:
- Around line 47-55: The same copy-pasted helpers are still duplicated in
JobsSection, so extract both formatAmount and StatusBlock into a shared utility
under app/src/agentworld/utils/ and update JobsSection to import them instead of
defining them inline. Reuse the existing relativeTime utility pattern as the
model, and make sure the shared exports are then used by LedgerSection and
JobsSection so the two files no longer diverge.
In `@app/src/agentworld/pages/LedgerSection.tsx`:
- Around line 72-87: The StatusBlock helper is duplicated across multiple page
components, so extract it into a shared reusable component like the existing
relativeTime helper. Move the shared rendering logic from StatusBlock into one
common location, then update LedgerSection, FeedSection, and JobsSection to
import and use that shared component while keeping the same props shape (tone,
title, body).
🪄 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: 58956fc2-9a90-4540-807c-b06f0b37f47c
📒 Files selected for processing (5)
app/src/agentworld/pages/FeedSection.tsxapp/src/agentworld/pages/JobsSection.tsxapp/src/agentworld/pages/LedgerSection.tsxapp/src/agentworld/utils/relativeTime.test.tsapp/src/agentworld/utils/relativeTime.ts
1 participant
Summary
relativeTimehelper intoapp/src/agentworld/utils/relativeTime.tsProblem
relativeTimewas copy-pasted identically in three filesSolution
agentworld/utils/relativeTime.tsRelated
Summary by CodeRabbit