Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR introduces a new Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
yashmehrotra
force-pushed
the
agent-health
branch
from
52f4aca to
c425c8e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/Canary/CanaryTabs.tsx`:
- Around line 153-160: agentTabs is being keyed by label (computed as name ||
id) causing collisions when two agents share the same name; update the
population logic in the agentMap.forEach loop to key agentTabs by the unique id
(use id as the object key) and keep label (name || id) only as the display
property (i.e., set agentTabs[id] = { key: id, value: id, label }); ensure any
downstream consumers that previously used the label as the key now use the agent
id (references: agentTabs, agentMap, label, id).
🧹 Nitpick comments (5)
src/components/Canary/index.tsx (2)
169-175: Unnecessary non-null assertion oncheck.agent_id.Line 170 uses
check.agent_id!butagent_idcan beundefinedfor local checks. It's harmless here becauseMap.get(undefined)returnsundefinedand the guard on line 171 handles it, but the!is misleading. Consider dropping it.Suggested fix
- const agent = agentMap.get(check.agent_id!); + const agent = agentMap.get(check.agent_id);
137-190: Agent fetch adds a sequential network call on every refresh cycle.
getAgentByIDsis awaited insidehandleFetch, creating a waterfall on each refresh interval. For now this is acceptable since the call is conditional and batched, but consider caching agent data separately (e.g., viauseQueryor a simple in-memory cache) so that repeated refreshes don't re-fetch the same agents.src/components/Canary/Columns/index.tsx (1)
151-168: Namespace quote-stripping is repeated across files.The
.replace(/"/g, "")pattern for sanitizing namespace strings appears here (lines 151, 157, 166) and also inCheckTitle.tsx(lines 20, 61). Consider extracting a small helper (e.g.,sanitizeNamespace) to DRY this up.src/components/Canary/CanaryTabs.tsx (1)
135-141: Query key with spread array may produce unstable keys.Line 136 spreads
agentIDsToFetchinto the query key. If the order of IDs changes between renders (e.g., due toSetiteration order varying with insertion order of different fetches), it could cause unnecessary refetches. Consider sorting the IDs for a stable key.Suggested fix
- ["db", "agents", ...agentIDsToFetch], + ["db", "agents", ...agentIDsToFetch.slice().sort()],src/components/Canary/CanaryPopup/CheckTitle.tsx (1)
61-61: Use the already-sanitizednamespacevariable instead of re-sanitizing.Line 20 assigns
const namespace = check?.namespace?.replace(/"/g, ""), but line 61 repeats the.replace(...)oncheck?.namespaceinstead of reusingnamespace. Use the variable for consistency.Suggested fix
- <Badge text={check?.namespace?.replace(/"/g, "") ?? ""} /> + <Badge text={namespace ?? ""} />
| const agentTabs = {} as Record< | ||
| string, | ||
| { key: string; value: string; label: string } | ||
| >; | ||
| agentMap.forEach((name, id) => { | ||
| const label = name || id; | ||
| agentTabs[label] = { key: label, value: id, label }; | ||
| }); |
There was a problem hiding this comment.
Potential tab collision when two agents share the same name.
Line 159 keys the agentTabs object by label (which is name || id). If two distinct agents have the same name, the second will overwrite the first, silently losing a tab. Consider keying by id instead and using label only for display.
Suggested fix
- agentMap.forEach((name, id) => {
- const label = name || id;
- agentTabs[label] = { key: label, value: id, label };
- });
+ agentMap.forEach((name, id) => {
+ const label = name || id;
+ agentTabs[id] = { key: id, value: id, label };
+ });📝 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.
| const agentTabs = {} as Record< | |
| string, | |
| { key: string; value: string; label: string } | |
| >; | |
| agentMap.forEach((name, id) => { | |
| const label = name || id; | |
| agentTabs[label] = { key: label, value: id, label }; | |
| }); | |
| const agentTabs = {} as Record< | |
| string, | |
| { key: string; value: string; label: string } | |
| >; | |
| agentMap.forEach((name, id) => { | |
| const label = name || id; | |
| agentTabs[id] = { key: id, value: id, label }; | |
| }); |
🤖 Prompt for AI Agents
In `@src/components/Canary/CanaryTabs.tsx` around lines 153 - 160, agentTabs is
being keyed by label (computed as name || id) causing collisions when two agents
share the same name; update the population logic in the agentMap.forEach loop to
key agentTabs by the unique id (use id as the object key) and keep label (name
|| id) only as the display property (i.e., set agentTabs[id] = { key: id, value:
id, label }); ensure any downstream consumers that previously used the label as
the key now use the agent id (references: agentTabs, agentMap, label, id).
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Performance