Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe chat UI supports unit recipients for direct messages and optional group names. The chat store queues active-channel markers, retries failed hub updates, and resynchronizes markers after reconnecting. New translations cover unit labels and optional group names. ChangesConversation creation updates
Active channel synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant NewConversationSheet
participant RecipientService
participant ChatAPI
NewConversationSheet->>RecipientService: load recipients when mode changes
RecipientService-->>NewConversationSheet: return people and units
NewConversationSheet->>ChatAPI: create conversation with TargetUserId or TargetUnitId
sequenceDiagram
participant ChatStore
participant ChatHub
participant Connection
ChatStore->>ChatHub: SetActiveChannel(channelId, null)
ChatHub-->>ChatStore: return success or failure
Connection->>ChatStore: report reconnect
ChatStore->>ChatHub: resynchronize active or null marker
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![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
🧹 Nitpick comments (4)
src/components/chat/new-conversation-sheet.tsx (2)
184-184: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExtract the recipient press handler.
filtered.mapcreates an anonymousonPresscallback for each row on every render. Extract each row into a memoized component and define its press handler withuseCallback.🤖 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 `@src/components/chat/new-conversation-sheet.tsx` at line 184, Extract the recipient row rendered by filtered.map into a memoized component, moving the Pressable and its recipient-specific behavior there. In that component, define the onPress handler with useCallback while preserving the existing dm versus toggle behavior and submitting-disabled state.Source: Coding guidelines
66-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd Jest coverage for
NewConversationSheet.Cover unit loading,
TargetUnitId,TargetUserId, blank group names, mode changes, and empty or error recipient states. Assert API arguments, thecreateAdHocChannelpayload, and rendered labels.🤖 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 `@src/components/chat/new-conversation-sheet.tsx` around lines 66 - 72, Add Jest tests for NewConversationSheet covering unit recipient loading, TargetUnitId and TargetUserId handling, blank group names, mode changes, and empty or failed recipient responses. Assert getRecipients arguments, the createAdHocChannel payload, and the rendered recipient labels across these scenarios.Source: Coding guidelines
src/stores/chat/__tests__/hub-invoke-args.test.ts (2)
75-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for reconnect re-reporting.
These assertions cover
setActiveChannelonly. They do not exercisehandleChatConnectedinsrc/stores/chat/store.ts, Lines 856-858. Add a Jest test that sets an active channel, callshandleChatConnected, and verifies theSetActiveChannelarguments.As per coding guidelines, “Generate tests for all components, services and logic generated. Ensure tests run without errors and fix any issues.”
🤖 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 `@src/stores/chat/__tests__/hub-invoke-args.test.ts` around lines 75 - 83, The existing test only verifies setActiveChannel; add coverage for handleChatConnected reconnect behavior. In the chat store test, set an active channel, invoke handleChatConnected, and assert mockInvoke receives chatHub, SetActiveChannel, the active channel, and null.Source: Coding guidelines
11-11: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument the
SetActiveChannelclear contract.
ChatHub.SetActiveChannelacceptsnullor an emptychannelIdto clear the active marker. Update the test comment to usestring? channelId.🤖 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 `@src/stores/chat/__tests__/hub-invoke-args.test.ts` at line 11, Update the `SetActiveChannel` test comment to declare `string? channelId`, documenting that the channel identifier may be null when clearing the active marker; leave the `asUnitId` parameter unchanged.
🤖 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 `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 66-72: Update the recipient loading and display logic in the
new-conversation sheet to use recipient-neutral or DM-specific translation keys
whenever includeUnits is true, replacing chat.search_people, chat.no_people, and
chat.load_people_failed in that mode. Add the corresponding keys and
translations to every locale while preserving the existing people-only copy for
non-DM modes.
- Around line 107-108: Validate targetId in the createDirectMessage path for
unit recipients before constructing the request, requiring a complete decimal
integer rather than relying on parseInt’s prefix parsing. Reject malformed
values such as “12abc” or “12.5” and avoid sending TargetUnitId when validation
fails; preserve the existing user-recipient request path.
In `@src/stores/chat/store.ts`:
- Around line 288-290: Update reset to explicitly invoke SetActiveChannel with
null arguments when clearing chat state, ensuring the marker is cleared even if
disconnectChatHub or connection.stop() fails. Reuse the existing safeInvoke
mechanism and preserve the active-channel reporting behavior elsewhere.
---
Nitpick comments:
In `@src/components/chat/new-conversation-sheet.tsx`:
- Line 184: Extract the recipient row rendered by filtered.map into a memoized
component, moving the Pressable and its recipient-specific behavior there. In
that component, define the onPress handler with useCallback while preserving the
existing dm versus toggle behavior and submitting-disabled state.
- Around line 66-72: Add Jest tests for NewConversationSheet covering unit
recipient loading, TargetUnitId and TargetUserId handling, blank group names,
mode changes, and empty or failed recipient responses. Assert getRecipients
arguments, the createAdHocChannel payload, and the rendered recipient labels
across these scenarios.
In `@src/stores/chat/__tests__/hub-invoke-args.test.ts`:
- Around line 75-83: The existing test only verifies setActiveChannel; add
coverage for handleChatConnected reconnect behavior. In the chat store test, set
an active channel, invoke handleChatConnected, and assert mockInvoke receives
chatHub, SetActiveChannel, the active channel, and null.
- Line 11: Update the `SetActiveChannel` test comment to declare `string?
channelId`, documenting that the channel identifier may be null when clearing
the active marker; leave the `asUnitId` parameter unchanged.
🪄 Autofix
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: a87efc22-aca8-429f-8ffd-2fc3257118d7
📒 Files selected for processing (12)
src/components/chat/new-conversation-sheet.tsxsrc/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
| // DM mode also offers units (Dispatch can open a 1:1 with a unit); | ||
| // group membership only supports users, so group mode stays people-only. | ||
| const includeUnits = mode === 'dm'; | ||
| getRecipients(true, includeUnits) | ||
| .then((result) => { | ||
| if (cancelled) return; | ||
| setRecipients((result.Data ?? []).filter(isPersonRecipient)); | ||
| setRecipients((result.Data ?? []).filter((r) => isPersonRecipient(r) || (includeUnits && isUnitRecipient(r)))); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use recipient-neutral copy in direct-message mode.
When includeUnits is true, the list contains people and units. The component still uses chat.search_people, chat.no_people, and chat.load_people_failed. A unit-only result then shows incorrect text. Add recipient-neutral keys or select mode-specific keys, and add them to every locale.
🤖 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 `@src/components/chat/new-conversation-sheet.tsx` around lines 66 - 72, Update
the recipient loading and display logic in the new-conversation sheet to use
recipient-neutral or DM-specific translation keys whenever includeUnits is true,
replacing chat.search_people, chat.no_people, and chat.load_people_failed in
that mode. Add the corresponding keys and translations to every locale while
preserving the existing people-only copy for non-DM modes.
| const targetId = recipientUserId(recipient); | ||
| const response = await createDirectMessage(isUnitRecipient(recipient) ? { TargetUnitId: parseInt(targetId, 10) } : { TargetUserId: targetId }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg --glob '!node_modules/**' --glob '!dist/**' -n -C 8 \
'TargetUnitId|CreateDirectMessageInput|recipientUserId|RecipientsResultData|getRecipients' .Repository: Resgrid/Dispatch
Length of output: 17490
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- relevant implementation ---'
sed -n '1,125p' src/components/chat/new-conversation-sheet.tsx
printf '%s\n' '--- recipient model and API types ---'
cat src/models/v4/messages/recipientsResultData.ts
sed -n '1,90p' src/models/v4/chat/chatInputs.ts
printf '%s\n' '--- repository references to recipient ID formats and unit targets ---'
rg --glob '!node_modules/**' --glob '!dist/**' -n -C 3 \
'includeUnits|RecipientsResultData|TargetUnitId|CreateDirectMessage|GetRecipients|Id: "[^"]*[:][^"]*"' .Repository: Resgrid/Dispatch
Length of output: 15783
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
node - <<'JS'
function recipientUserId(id) {
return id.includes(':') ? (id.split(':').pop() ?? id) : id;
}
const cases = ['U:12', 'U:12abc', 'U:12.5', 'U:', 'U:abc', '12', ' 12 '];
for (const raw of cases) {
const targetId = recipientUserId(raw);
const parsed = Number.parseInt(targetId, 10);
const isDecimalInteger = /^[0-9]+$/.test(targetId);
console.log(JSON.stringify({ raw, targetId, parsed, isDecimalInteger }));
}
JS
printf '%s\n' '--- repository origin ---'
git remote get-url origin 2>/dev/null || trueRepository: Resgrid/Dispatch
Length of output: 706
Validate the unit ID before building the request.
recipientUserId can produce malformed values, and parseInt accepts numeric prefixes such as "12abc" or "12.5" as 12. Require a decimal integer before sending TargetUnitId; otherwise reject the recipient.
🤖 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 `@src/components/chat/new-conversation-sheet.tsx` around lines 107 - 108,
Validate targetId in the createDirectMessage path for unit recipients before
constructing the request, requiring a complete decimal integer rather than
relying on parseInt’s prefix parsing. Reject malformed values such as “12abc” or
“12.5” and avoid sending TargetUnitId when validation fails; preserve the
existing user-recipient request path.
| // Report the actively viewed conversation to the hub so the server can | ||
| // suppress push notifications for it; null clears the marker. | ||
| void safeInvoke('SetActiveChannel', channelId ?? null, null); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'useChatStore.*reset|chat.*reset' src
rg -n -C 8 'disconnectFromHub|disconnectAll|\.stop\(\)' srcRepository: Resgrid/Dispatch
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked candidate files ---'
git ls-files 'src/stores/chat/store.ts' 'src' | rg '(^|/)(chat|hub|store)(/|[^/]*)' | head -n 200
printf '%s\n' '--- store outline ---'
ast-grep outline src/stores/chat/store.ts
printf '%s\n' '--- store lifecycle references ---'
rg -n -C 12 --glob '!src/utils/webview-scripts.ts' \
'SetActiveChannel|safeInvoke|activeChannelId|(^|[^[:alnum:]_])reset[[:space:]]*[:=]|\breset\s*\(' \
src/stores/chat/store.ts
printf '%s\n' '--- exact reset and hub lifecycle call sites ---'
rg -n -C 8 --glob '!src/utils/webview-scripts.ts' \
'(useChatStore|chatStore|ChatStore|disconnectFromHub|disconnectAll|startHub|stopHub|connectToHub|hub\.stop|\.stop\(\)|\.reset\(\))' \
src --glob '*.{ts,tsx,js,jsx}'Repository: Resgrid/Dispatch
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
store = Path("src/stores/chat/store.ts")
lines = store.read_text().splitlines()
for start, end in ((250, 305), (850, 900)):
print(f"--- {store}:{start}-{end} ---")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number}:{lines[number-1]}")
patterns = re.compile(
r"(useChatStore|chatStore|disconnectFromHub|disconnectAll|"
r"startHub|stopHub|connectToHub|SetActiveChannel|activeChannelId|"
r"\.reset\s*\(|\breset\s*[:=])"
)
excluded = {"src/utils/webview-scripts.ts"}
print("--- bounded tracked-source matches ---")
count = 0
for path_text in sorted(Path("src").rglob("*")):
if not path_text.is_file() or str(path_text) in excluded:
continue
if path_text.suffix not in {".ts", ".tsx", ".js", ".jsx"}:
continue
for number, line in enumerate(path_text.read_text(errors="replace").splitlines(), 1):
if patterns.search(line):
print(f"{path_text}:{number}:{line}")
count += 1
if count >= 300:
raise SystemExit
PYRepository: Resgrid/Dispatch
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start=1, end=None):
lines = Path(path).read_text(errors="replace").splitlines()
end = end or len(lines)
print(f"--- {path}:{start}-{min(end, len(lines))} ---")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number}:{lines[number-1]}")
show("src/app/(app)/_layout.tsx")
show("src/hooks/useSignalR.ts")
PY
printf '%s\n' '--- tracked SignalR and hub paths ---'
git ls-files | rg -i '(^|/)(signalr|hub|.*signal.*|.*hub.*)(/|$|\.)' | head -n 100Repository: Resgrid/Dispatch
Length of output: 32169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
targets = [
"src/stores/signalr/signalr-store.ts",
"src/services/signalr.service.ts",
"src/hooks/use-signalr-lifecycle.ts",
]
patterns = re.compile(
r"(connectChatHub|disconnectChatHub|connectToHub|disconnectFromHub|"
r"safeInvoke|SetActiveChannel|stop\(|start\(|isConnected|disconnect)"
)
for name in targets:
path = Path(name)
lines = path.read_text(errors="replace").splitlines()
print(f"--- {name} matching regions ---")
hits = [i for i, line in enumerate(lines) if patterns.search(line)]
shown = set()
for hit in hits:
start, end = max(0, hit - 8), min(len(lines), hit + 12)
key = (start, end)
if key in shown:
continue
shown.add(key)
print(f"[{start+1}-{end}]")
for number in range(start + 1, end + 1):
print(f"{number}:{lines[number-1]}")
PY
printf '%s\n' '--- production reset call sites excluding generated source ---'
python3 - <<'PY'
from pathlib import Path
for path in sorted(Path("src").rglob("*")):
if not path.is_file() or path.suffix not in {".ts", ".tsx", ".js", ".jsx"}:
continue
if path.name in {"webview-scripts.ts", "form-render-source.ts"}:
continue
for number, line in enumerate(path.read_text(errors="replace").splitlines(), 1):
if "useChatStore.getState().reset" in line or "useChatStore.reset" in line:
print(f"{path}:{number}:{line}")
PYRepository: Resgrid/Dispatch
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
layout = Path("src/app/(app)/_layout.tsx").read_text()
store = Path("src/stores/chat/store.ts").read_text()
service = Path("src/services/signalr.service.ts").read_text()
teardown_start = layout.index("async function teardownSignedInSession")
teardown = layout[teardown_start:layout.index("\n}\n", teardown_start) + 2]
print("--- lifecycle invariants ---")
print("chat disconnect precedes reset:", teardown.index("disconnectChatHub") < teardown.index("useChatStore.getState().reset"))
print("reset call count in teardown:", teardown.count("useChatStore.getState().reset"))
print("disconnectChatHub awaits service disconnect:", bool(re.search(
r"disconnectChatHub:\s*async\s*\(\)\s*=>.*?await signalRService\.disconnectFromHub",
store, re.S
)))
print("disconnect service awaits connection.stop:", bool(re.search(
r"public async disconnectFromHub\(hubName: string\).*?await connection\.stop\(\)",
service, re.S
)))
print("disconnectChatHub catches errors without rethrow:", bool(re.search(
r"disconnectChatHub:\s*async\s*\(\)\s*=>.*?catch \(error\).*?set\(\{ error: err \}\);\s*\n\s*}\s*,",
store, re.S
)))
print("--- production reset call sites ---")
for path in sorted(Path("src").rglob("*")):
if not path.is_file() or path.name in {"webview-scripts.ts", "form-render-source.ts"}:
continue
if path.suffix not in {".ts", ".tsx", ".js", ".jsx"}:
continue
for number, line in enumerate(path.read_text(errors="replace").splitlines(), 1):
if "useChatStore.getState().reset" in line or "useChatStore.reset" in line:
print(f"{path}:{number}:{line.strip()}")
print("--- SetActiveChannel call sites ---")
for path in sorted(Path("src").rglob("*")):
if not path.is_file() or path.name in {"webview-scripts.ts", "form-render-source.ts"}:
continue
if path.suffix not in {".ts", ".tsx", ".js", ".jsx"}:
continue
for number, line in enumerate(path.read_text(errors="replace").splitlines(), 1):
if "SetActiveChannel" in line:
print(f"{path}:{number}:{line.strip()}")
PYRepository: Resgrid/Dispatch
Length of output: 1267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
lines = Path(path).read_text(errors="replace").splitlines()
print(f"--- {path}:{start}-{end} ---")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number}:{lines[number-1]}")
show("src/stores/chat/store.ts", 1, 90)
show("src/stores/chat/__tests__/hub-invoke-args.test.ts", 1, 115)
show("src/stores/signalr/signalr-store.ts", 817, 833)
show("src/services/signalr.service.ts", 839, 894)
PYRepository: Resgrid/Dispatch
Length of output: 11164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 'function safeInvoke|const safeInvoke|safeInvoke\s*=' src/stores/chat/store.tsRepository: Resgrid/Dispatch
Length of output: 894
Clear the active-channel marker in reset.
disconnectChatHub swallows disconnect failures, so reset can run after connection.stop() fails. In that case, reset does not send SetActiveChannel(null, null), and the server marker can remain stale.
🤖 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 `@src/stores/chat/store.ts` around lines 288 - 290, Update reset to explicitly
invoke SetActiveChannel with null arguments when clearing chat state, ensuring
the marker is cleared even if disconnectChatHub or connection.stop() fails.
Reuse the existing safeInvoke mechanism and preserve the active-channel
reporting behavior elsewhere.
| if (selected.size === 0) return; | ||
| setSubmitting(true); | ||
| try { | ||
| // Name is optional — the server auto-names the group after its members. | ||
| const response = await createAdHocChannel({ Name: groupName.trim(), MemberUserIds: Array.from(selected) }); |
There was a problem hiding this comment.
WHAT: createGroup now sends Name: '' (empty string) when no group name is entered, because the !groupName.trim() guard was removed. WHY: an empty string is a distinct wire value from an omitted field — if the server auto-names based on field absence (or requires non-empty), sending "" either creates a blank-named group or returns a 400, defeating the optional-name feature. HOW: make Name optional in CreateAdHocChannelInput and omit it when blank, or confirm the server treats "" as the auto-name trigger.
// Make Name optional in CreateAdHocChannelInput and only send it when provided:
const input: CreateAdHocChannelInput = { MemberUserIds: Array.from(selected) };
const name = groupName.trim();
if (name) input.Name = name;
const response = await createAdHocChannel(input);Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 124 to 128:
WHAT: createGroup now sends `Name: ''` (empty string) when no group name is entered, because the `!groupName.trim()` guard was removed. WHY: an empty string is a distinct wire value from an omitted field — if the server auto-names based on field absence (or requires non-empty), sending `""` either creates a blank-named group or returns a 400, defeating the optional-name feature. HOW: make `Name` optional in CreateAdHocChannelInput and omit it when blank, or confirm the server treats `""` as the auto-name trigger.
Suggested Code:
// Make Name optional in CreateAdHocChannelInput and only send it when provided:
const input: CreateAdHocChannelInput = { MemberUserIds: Array.from(selected) };
const name = groupName.trim();
if (name) input.Name = name;
const response = await createAdHocChannel(input);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| set({ activeChannelId: channelId }); | ||
| // Report the actively viewed conversation to the hub so the server can | ||
| // suppress push notifications for it; null clears the marker. | ||
| void safeInvoke('SetActiveChannel', channelId ?? null, null); |
There was a problem hiding this comment.
WHAT: The promise returned by safeInvoke is discarded with the void operator and has no .catch handler. WHY: If safeInvoke rejects, the rejection is unhandled, violating the rule that every async operation must be guarded. HOW: Either attach a .catch (e.g. safeInvoke(...).catch(err => logger.error('SetActiveChannel failed', { channelId, err }))) or wrap in try/catch if using await.
Also found in:
src/stores/chat/store.ts:858-858
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/stores/chat/store.ts:
Line 290:
WHAT: The promise returned by safeInvoke is discarded with the `void` operator and has no .catch handler. WHY: If safeInvoke rejects, the rejection is unhandled, violating the rule that every async operation must be guarded. HOW: Either attach a .catch (e.g. `safeInvoke(...).catch(err => logger.error('SetActiveChannel failed', { channelId, err }))`) or wrap in try/catch if using await.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/stores/chat/__tests__/hub-invoke-args.test.ts`:
- Line 168: Update the SetActiveChannel assertion in the hub invocation test to
explicitly verify that mockInvoke was not called with null as either argument,
replacing the expect.anything() matchers with null values.
🪄 Autofix
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: 8a04a512-9452-410b-85d6-07b5eecb7c98
📒 Files selected for processing (2)
src/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.ts
| useChatStore.getState().handleChatConnected(); | ||
| await flush(); | ||
|
|
||
| expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify Jest documents that expect.anything() excludes null and undefined.
curl -fsSL https://jestjs.io/docs/expect |
rg -i -C 2 'anything.*null|null.*anything'Repository: Resgrid/Dispatch
Length of output: 37902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/stores/chat/__tests__/hub-invoke-args.test.ts"
printf '%s\n' '--- test context ---'
cat -n "$file" | sed -n '130,185p'
printf '%s\n' '--- related symbols and calls ---'
rg -n -C 3 'handleChatConnected|SetActiveChannel|mockInvoke' src/stores/chatRepository: Resgrid/Dispatch
Length of output: 12193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package and Jest configuration ---'
rg -n -C 2 '"jest"|jest.config|testMatch|testRegex' package.json '*jest*' 2>/dev/null || true
printf '%s\n' '--- exact test structure ---'
cat -n src/stores/chat/__tests__/hub-invoke-args.test.ts | sed -n '1,210p'
printf '%s\n' '--- handler definitions and call sites ---'
rg -n -C 8 'handleChatConnected|SetActiveChannel|mockInvoke' src --glob '*.{ts,tsx}'Repository: Resgrid/Dispatch
Length of output: 25762
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/stores/chat/__tests__/hub-invoke-args.test.ts")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if "SetActiveChannel" in line or "handleChatConnected" in line:
print(f"{i}: {line}")
PY
printf '%s\n' '--- tracked candidate files ---'
git ls-files | rg '(^|/)(chat|hub).*|src/stores/chat' | head -80Repository: Resgrid/Dispatch
Length of output: 1896
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/stores/chat/__tests__/hub-invoke-args.test.ts"
cat -n "$file" | sed -n '130,185p'
printf '\n--- related code ---\n'
rg -n -C 8 'handleChatConnected|SetActiveChannel|mockInvoke' src/stores/chatRepository: Resgrid/Dispatch
Length of output: 17346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete reconnect handler ---'
cat -n src/stores/chat/store.ts | sed -n '860,890p'
printf '%s\n' '--- matcher behavior for the calls relevant to this test ---'
node - <<'JS'
const anything = value => value !== null && value !== undefined;
const calls = [
['chatHub', 'SetActiveChannel', null, null],
['chatHub', 'SetActiveChannel', 'channel-1', null],
];
const matchesOriginal = call =>
call[0] === 'chatHub' &&
call[1] === 'SetActiveChannel' &&
anything(call[2]) &&
anything(call[3]);
const matchesNullFix = call =>
call[0] === 'chatHub' &&
call[1] === 'SetActiveChannel' &&
call[2] === null &&
call[3] === null;
for (const call of calls) {
console.log(JSON.stringify(call), {
originalNot: !matchesOriginal(call),
nullFixNot: !matchesNullFix(call),
});
}
JSRepository: Resgrid/Dispatch
Length of output: 1791
Assert that the null marker is not resent.
expect.anything() excludes null, so this assertion does not detect SetActiveChannel(null, null). Use null for both arguments.
🤖 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 `@src/stores/chat/__tests__/hub-invoke-args.test.ts` at line 168, Update the
SetActiveChannel assertion in the hub invocation test to explicitly verify that
mockInvoke was not called with null as either argument, replacing the
expect.anything() matchers with null values.
Source: Coding guidelines
2 participants
Pull Request Description
RG-T117 Chat fix
This PR introduces several improvements to the chat functionality:
New Features & Enhancements
Direct messages with units: Users can now start 1:1 conversations with units (not just personnel). When starting a DM with a unit, the request is sent with
TargetUnitIdinstead ofTargetUserId. Units are visually distinguished with a truck icon and a "Unit" badge instead of a user avatar.Optional group name: Group chat creation no longer requires a name. The server auto-names groups based on their members. The placeholder text and validation were updated accordingly.
Active channel notification suppression: The app now reports the currently viewed conversation to the chat hub via a new
SetActiveChannelinvocation, allowing the server to suppress push notifications for the conversation the user is actively viewing. This state is also re-established automatically after connection drops/reconnects.Bug Fixes
modeto the effect dependency array so recipients are properly refreshed when switching between DM and group modes.Summary by CodeRabbit
New Features
Bug Fixes