feat(call): add call to soup#2559
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbitNew Features
WalkthroughThis pull request introduces comprehensive support for call records as a new entity type. It adds a "calls" list view with routing, filtering, and sidebar navigation on the frontend; extends the backend call domain with record querying and channel-name resolution; and integrates call-specific soup models and filters throughout the application infrastructure. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
whutchinson98
force-pushed
the
hutch/feat-call-in-soup
branch
2 times, most recently
from
c648516 to
9b3d2c9
Compare
whutchinson98
force-pushed
the
hutch/feat-call-in-soup
branch
from
9b3d2c9 to
627b0f0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
js/app/packages/macro-entity/src/queries/rename.ts (1)
62-75:⚠️ Potential issue | 🟠 MajorThe new
callskip path still fails the rename flow.
bulkRenameMutationFnstill runsvalidateEntityRename, which throws on calls before this branch executes. If that check is relaxed later,return { success: false }will still tripbulkRenameOnSettledand surface a toast/rollback for an intentionally skipped entity. Filter calls out before validation/mutation, or treat them as a no-op success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/macro-entity/src/queries/rename.ts` around lines 62 - 75, The skip-for-`call` path currently returns { success: false } which triggers bulkRenameOnSettled rollback/toast and still allows validateEntityRename to run elsewhere; update the flow so calls are treated as no-op successes or filtered before validation: either (A) change performEntityRename to return { success: true } when getEntityRenameData(operation) is null (so skipped `call` operations do not surface errors), or (B) filter out operations with entity.type === 'call' before calling bulkRenameMutationFn/validateEntityRename so validateEntityRename is never invoked for calls; refer to getEntityRenameData, performEntityRename, bulkRenameMutationFn, validateEntityRename, and bulkRenameOnSettled when applying the fix.js/app/packages/app/component/command/CommandMenu.tsx (1)
159-173:⚠️ Potential issue | 🟡 Minor
callselection becomes a silent no-op in command menu.
callitems skip the open path and then fall through toCommandState.close(), so selecting a visible result does nothing except close the menu. Please handlecallexplicitly (navigate, block with feedback, or keep menu open) instead of silent close.Suggested minimal fix (avoid silent close for call items)
- if (isEntityItem(item) && item.data.type !== 'call') { + if (isEntityItem(item) && item.data.type === 'call') { + // TODO: add explicit call navigation/behavior when available. + return; + } + + if (isEntityItem(item)) { const blockName = itemToBlockName(item.data); if (blockName) { openWithSplit( { type: blockName, id: item.id }, { referredFrom: 'kommand-menu', preferNewSplit: openInNewSplit, } ); } CommandState.close(); CommandState.setQuery(''); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/component/command/CommandMenu.tsx` around lines 159 - 173, The current selection logic treats isEntityItem items with data.type === 'call' as a silent no-op because CommandState.close() and CommandState.setQuery('') run unconditionally; update the CommandMenu selection handling so that calls are handled explicitly instead of falling through: inside the block that checks isEntityItem(item), add an explicit branch for item.data.type === 'call' (or an else) and either perform the intended navigation (e.g., call openWithSplit with the correct target), show a user feedback/error, or return without closing the menu; move the CommandState.close() and CommandState.setQuery('') calls into the branch that handles non-call blockName opens (the branch that currently calls openWithSplit) so call items do not trigger a silent close. Ensure you modify the logic around isEntityItem, itemToBlockName, openWithSplit, CommandState.close, and CommandState.setQuery accordingly.js/app/packages/queries/soup/normalized-cache/operations.ts (1)
253-284:⚠️ Potential issue | 🟠 Major
callRecordrefetches can't surface uncached items.Returning
nullhere routesrefetchSoupEntity()toinvalidateSoupEntity(entityId). That only reaches queries already linked tosoup:${entityId}, so a brand-new call record will not appear in an active/callsfeed until something broader invalidates soup.Minimal fallback if single-item call fetches are unsupported
- if (!filter) { - invalidateSoupEntity(entityId); + if (!filter) { + if (entityType === 'callRecord') { + invalidateAllSoup(); + } else { + invalidateSoupEntity(entityId); + } return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/queries/soup/normalized-cache/operations.ts` around lines 253 - 284, buildSingleEntityFilter currently returns null for entityType 'callRecord', which causes refetchSoupEntity to call invalidateSoupEntity(entityId) and only invalidate the specific soup:${entityId} key, preventing new call records from surfacing; change the 'callRecord' branch in buildSingleEntityFilter to return the generic base PostSoupRequest (the variable base already defined) as a fallback so refetchSoupEntity will issue a broader soup query invalidation (e.g., the unfiltered/placeholder request) instead of null; reference buildSingleEntityFilter, the 'callRecord' .with branch, and invalidateSoupEntity/refetchSoupEntity when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/app-sidebar/sidebar.tsx`:
- Around line 350-356: The hotkey for CALLS_LINK isn't being registered because
registerSidebarHotkeys iterates only over the static SIDEBAR_LINKS; update
registration to include the dynamic visibleLinks() (or explicitly include
CALLS_LINK when calls are enabled) so its hotkey 'l' is wired. Specifically,
modify where registerSidebarHotkeys is called to pass the full computed list
(visibleLinks() or SIDEBAR_LINKS.concat(CALLS_LINK) when enabled) or add a
separate registration for CALLS_LINK.hotkey pointing to navigation to
LIST_VIEW_PATHS.calls; ensure registerSidebarHotkeys (and any handlers it
installs) uses the link objects' id/hotkey/href to bind the key action.
In `@js/app/packages/app/component/mobile/MobileSearch.tsx`:
- Line 110: The guard using isEntityItem(item) && item.data.type !== 'call'
prevents call-type entities from triggering navigation (openWithSplit) and only
runs the close/reset path, causing taps on call results to be no-ops; update the
selection logic in MobileSearch (the branch that currently checks isEntityItem
and item.data.type) to either remove the type !== 'call' exclusion or add an
explicit case for item.data.type === 'call' that invokes openWithSplit(...) (or
the same navigation helper used for other entity types) before performing any
close/reset cleanup so call entries navigate correctly; ensure the change also
covers the alternate branch referenced around the other occurrence (the same
conditional near the close/reset logic).
In `@js/app/packages/app/component/next-soup/filters/query-filters.ts`:
- Line 67: The `.with({ tag: 'callRecord' }, () => true)` branch currently
bypasses channel filtering; change it to mirror other item types by invoking the
same filter check — call `isIdFilteredOut` with `body.call_filters?.channel_ids`
and the record's channel id (e.g., `data.channelId`) and return the negated
result so `callRecord` items are excluded when their channel id is filtered out;
update the `.with` handler for the 'callRecord' tag in query-filters (use the
same pattern as other tags).
- Around line 157-164: Add call_filters to the baseline filters and apply call
filtering in the item filter: update QUERY_FILTERS_BASE to include call_filters:
{ call_ids: EXCLUDE } so calls are excluded by default, and change
filterSoupItemByRequestBody to handle the callRecord tag the same way as other
entities by replacing the unconditional true with a check using
isIdFilteredOut(body.call_filters?.call_ids, data.id) so callRecord items are
filtered according to body.call_filters.
In `@js/app/packages/block-canvas/component/CanvasController.tsx`:
- Around line 864-867: The TODO comment near the checks on entityType in
CanvasController.tsx is outdated: update the comment to indicate both
unsupported types are skipped. Modify the TODO to mention both 'channel_message'
and 'call' (or replace with a clearer note like "TODO: add support for
channel_message and call") next to the existing conditional checks (entityType
=== 'channel_message' and entityType === 'call') so it accurately reflects
current behavior.
In `@js/app/packages/entity/src/extractors-property/entity-key-properties.tsx`:
- Line 34: The current mapping (.with({ type: 'call' }, () =>
EntityType.CHANNEL)) incorrectly pairs a call record ID with EntityType.CHANNEL;
update the getEntityType mapping (or the extractor that contains .with({ type:
'call' }, ...)) so calls are not mapped to CHANNEL by default. Either
remove/mark the 'call' branch as unsupported (so saveMutation won't send a
CHANNEL mutation for a call ID), or change the mapping to return a pair {
entityType, entityId } (or provide an entityId selector) so that when type ===
'call' the entityType is CHANNEL but the entityId is taken from channelId
instead of props.entity.id; adjust callers accordingly
(saveMutation/entityType() usage) to consume the new pair if you choose the
paired approach.
In `@js/app/packages/macro-entity/src/queries/dss.ts`:
- Around line 33-35: The current filter that excludes 'call' from the optimistic
removal is ineffective because mutationFn still rejects calls via
isUnsupportedEntity, causing mixed selections to fail; fix this by building a
single deletableEntities array (e.g., const deletableEntities =
entities.filter(e => e.type !== 'channel_message' && e.type !== 'call' &&
!isUnsupportedEntity(e))) and then use that same deletableEntities list both in
onMutate (to remove only those items optimistically) and in mutationFn (to
perform deleteItem calls only for those items), ensuring calls are neither
attempted to be deleted nor optimistically removed.
In `@js/app/packages/macro-entity/src/queries/rename.ts`:
- Around line 206-208: The current .map(getEntityRenameData).filter(...)
pipeline creates an updates array that removes null entries and therefore shifts
indices, breaking the rollback alignment in bulkRenameOnSettled which expects
onMutateResult.updates[index] to correspond to the original params index; fix by
preserving the original operation index when building updates—e.g. have
getEntityRenameData (or the mapping step) return a structure like {index, data:
EntityRenameData | null} and then filter nulls while keeping index, or build a
map keyed by entity ID and have bulkRenameOnSettled look up by that key; update
any references to updates in bulkRenameOnSettled and onMutateResult to use the
preserved index or key so rollbacks target the correct entity.
In `@js/app/packages/queries/soup/normalized-cache/operations.ts`:
- Around line 91-100: Replace the switch in getSoupItemId with a ts-pattern
match to enforce exhaustive handling: use match(item).with({ tag: 'channel' },
() => item.data.channel.id).with({ tag: 'callRecord' }, () =>
item.data.callId).with({ tag: 'document' }, () => item.data.id).with({ tag:
'chat' }, () => item.data.id).with({ tag: 'project' }, () =>
item.data.id).with({ tag: 'emailThread' }, () => item.data.id).exhaustive();
mirror the style of getSearchResultId so each tag variant is explicitly handled
and the callRecord/channel extraction uses the nested fields while other
variants return item.data.id.
In `@js/app/packages/queries/soup/recently-viewed.ts`:
- Around line 40-48: The current mapping filters out items with item.tag ===
'callRecord' after the server-side fetch/limit (page.items), which can underflow
the intended number of recent non-call items; change the logic so the exclusion
of callRecord happens before any client-side limit is applied (ideally in the
query/request to the backend) or implement a loop to fetch additional pages
until you accumulate the desired count of non-call items; specifically update
the code handling page.items and the fetch that populates it so the filter for
item.tag !== 'callRecord' runs as part of the query or before any slicing/limit,
keeping the mapping that sets id and viewedAt intact.
In `@rust/cloud-storage/call/src/domain/channel_name.rs`:
- Around line 28-38: The code currently treats whitespace-only or empty strings
as valid names; change all name-handling paths (the channel_name mapping in the
organization/public/team branches, resolve_private_channel_name(), and
display_name) to normalize inputs by trimming and treating "" or all-whitespace
as None before composing or returning a label. Implement or use a small helper
(e.g., is_blank/normalize_name) to map Option<String> where Some(s) => if
s.trim().is_empty() then None else Some(s.trim().to_string()), apply this in the
channel_name.map(...) closures, in resolve_private_channel_name() when checking
participants, and in display_name so that fallbacks (e.g.,
"Organization"/"Public"/team fallbacks) are used when names are blank. Ensure
participant lists that are empty or produce only-blank names are treated as
missing so the fallback path is taken.
In `@rust/cloud-storage/call/src/outbound/pg_call_repo.rs`:
- Around line 739-797: The loop over rows does an N+1 query for participants;
change the logic in the function building CallRecord/CallRecordParticipant so
you first collect call_ids partitioned by is_active (from rows), run two batched
queries against call_participants and call_record_participants using WHERE
call_id = ANY($1) to fetch all participants at once, group the returned rows by
call_id into a map, and then when constructing each CallRecord (fields: call_id,
participants, etc.) pull the participants from the map instead of running
per-row queries; ensure you preserve ordering by joined_at when grouping and
handle empty id lists to avoid sending invalid SQL params.
In `@rust/cloud-storage/soup/src/domain/models.rs`:
- Around line 281-299: The build_call_request currently ignores
CursorWithValAndFilter's pagination fields (id, limit, val) for the Cursor
variant; update the call domain to support pagination by adding corresponding
cursor fields to GetCallRecordsRequest (e.g., cursor_id/id, cursor_limit/limit,
cursor_val/val) in call/src/domain/models.rs and then populate those fields
inside build_call_request (in the Cursor match arm) by passing
CursorWithValAndFilter.id, .limit and .val (similar to how
build_email_request/build_comms_request propagate cursor values), leaving the
Sort and Frecency behavior unchanged.
---
Outside diff comments:
In `@js/app/packages/app/component/command/CommandMenu.tsx`:
- Around line 159-173: The current selection logic treats isEntityItem items
with data.type === 'call' as a silent no-op because CommandState.close() and
CommandState.setQuery('') run unconditionally; update the CommandMenu selection
handling so that calls are handled explicitly instead of falling through: inside
the block that checks isEntityItem(item), add an explicit branch for
item.data.type === 'call' (or an else) and either perform the intended
navigation (e.g., call openWithSplit with the correct target), show a user
feedback/error, or return without closing the menu; move the
CommandState.close() and CommandState.setQuery('') calls into the branch that
handles non-call blockName opens (the branch that currently calls openWithSplit)
so call items do not trigger a silent close. Ensure you modify the logic around
isEntityItem, itemToBlockName, openWithSplit, CommandState.close, and
CommandState.setQuery accordingly.
In `@js/app/packages/macro-entity/src/queries/rename.ts`:
- Around line 62-75: The skip-for-`call` path currently returns { success: false
} which triggers bulkRenameOnSettled rollback/toast and still allows
validateEntityRename to run elsewhere; update the flow so calls are treated as
no-op successes or filtered before validation: either (A) change
performEntityRename to return { success: true } when
getEntityRenameData(operation) is null (so skipped `call` operations do not
surface errors), or (B) filter out operations with entity.type === 'call' before
calling bulkRenameMutationFn/validateEntityRename so validateEntityRename is
never invoked for calls; refer to getEntityRenameData, performEntityRename,
bulkRenameMutationFn, validateEntityRename, and bulkRenameOnSettled when
applying the fix.
In `@js/app/packages/queries/soup/normalized-cache/operations.ts`:
- Around line 253-284: buildSingleEntityFilter currently returns null for
entityType 'callRecord', which causes refetchSoupEntity to call
invalidateSoupEntity(entityId) and only invalidate the specific soup:${entityId}
key, preventing new call records from surfacing; change the 'callRecord' branch
in buildSingleEntityFilter to return the generic base PostSoupRequest (the
variable base already defined) as a fallback so refetchSoupEntity will issue a
broader soup query invalidation (e.g., the unfiltered/placeholder request)
instead of null; reference buildSingleEntityFilter, the 'callRecord' .with
branch, and invalidateSoupEntity/refetchSoupEntity when making this change.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7545f970-9b95-497d-8daa-d9c7abd56ba5
⛔ Files ignored due to path filters (108)
js/app/packages/service-clients/service-cognition/generated/tools/schemas.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/tools/types.tsis excluded by!**/generated/**js/app/packages/service-clients/service-search/generated/models/callFilters.tsis excluded by!**/generated/**js/app/packages/service-clients/service-search/generated/models/entityFilters.tsis excluded by!**/generated/**js/app/packages/service-clients/service-search/generated/models/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/callFilters.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/callRecord.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/callRecordChannelName.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/entityFilterAst.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/entityFilters.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecord.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecordChannelName.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecordDurationMs.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecordEndedAt.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecordParticipant.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupCallRecordParticipantLeftAt.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupItem.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupItemOneOfOneone.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/schemas/soupItemOneOfOneoneTag.tsis excluded by!**/generated/**js/app/packages/service-clients/service-storage/generated/zod.tsis excluded by!**/generated/**rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lockrust/cloud-storage/call/.sqlx/query-32d668d355465b6e54567a087cf54754567424b802606a92cf204041a9b70bf4.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-4fbbbf3257ecffd107a3d848ab980c51f5a28b25de618cd976b9871b0d6bcdec.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-6ca02c6511b3279813710cb93a275d215c3edeb60072739f71e5e7324772349c.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-6dae1be229aa02fd589a494c2219f9ffafb95d71284c9062109b4fc7ccf80206.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-7153394f8eb16ffb119e7cf8f3eb00791668bed040994bf21e00b397f4b05405.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-79b6d9d4f6b3469253e2fe01fef337bec8f6d447731718573f170ff4fa24b88d.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-7e26dfe09fa65aa0bad62337b51c2d22e4703c7fe2035725142f180193c35808.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-9b8eca330ae09809a79941d077aafafda5a2f65b86d94d3f517a34efa6d412a1.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-a7baf370108371fedbc9eec0287181a1581a3cc2c483b49f0b7579dd7338bd22.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-cd19706a12766971babf411d05ea10a1fe2cfc2898a8043d9d648c177ef9cb37.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-e233b0f2888b75bd70f9b6ecc11593c8be6cf5d43c234873dbc6f3b3296c8f00.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-fdce21902ca26481495bd105b110b6da30dde18159813f92012d47c820b1c5a5.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-03313f689cbc216503f0372eca83b82f58a55d5c030892b6889eb77e76dbae2e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-03a323aefdba22727c6789b28d90d7200e795a4591812dab4c4e7718c5512055.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-0e0422c73b3bdb5c60680cec7581ac905f34c70bd295694a120e8dd7891e3853.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-137986b2287f8a7e8d9023d6f9bfee888a6e18170f84df631df81e3e84158348.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-16fe6d8422135f9b01d5acb48576c9ebeea62fe8b80747ddcf9c5fdeee1a6c7e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-183fbda8d590393f7f3faa763c2cb6344b42556f949b59a0f8f77a01ec8572c4.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-21ac8784f90a5e311b658fe88195bdf9c83dfc84f0b5a8e8075a7153d7f41818.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-29496c090bdffe2b771ad704687525136fcd17a1d3e680142e5d0f9e253af578.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-2c1ca56683bc4ff97f5b9768e6da72ee6558c2ea8bb9b58237370e3cb80d309c.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-2e5e76597cf6fe734978fef0762c7c86aeba3b076e8684a85696045ed3a4636e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-359a16ea19cd02675cf7f6ffb815f76ad39c5aca7b6503ae8e43ebd1cc5d733f.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-37b5168f739c490a3373c65eab35f247a34af0a8e659cbad61cb99e3094e5a60.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-37c7a9fd9f1156ce7de8a691fab1d33b9b50bd1e3525338b49991091df258321.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-3a7be99469ed150e0506efa786b672198a6d3419c6bd5548418520cd3e6a8e95.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-3b147b8bcb7d08a87729fc68c562ea545e65d41bd61891ed3aad8eda3c4d90dd.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-3b570b222c479d824a07d83d5af9bb51833195bf60052e04a1c754df22f57220.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-44682e65711eb45a1dbec725646b7b0e02ced66e66f2d985f3da2d3eb7ad8c60.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-45064c055927af781bc06e3a602a9efbefbd111110b9f0fab5cf029e70208c1e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-45a55e0ea2f0cc2d7583ede119ffd4059933d25cca50775c74f1ef5a0224493c.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-48df79c41ba4ec7fa2454ac962aee9ff97875c9e2335b46fe13b2bbdee721186.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-4afa79ca95bdbcebe9d3de7ebb204636419e6cf89e650a8716c40ab2e588aa5b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-4b75a017d1bfeb600d0b75c58220b27c51a57253d0d82c55793eb5b0852d476f.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-4c695b011d49ba7d5fce4b309875ed952dc386ac760444d7883c5a76718f15be.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-4cc88f2a38cb0722282ef262adefd9fcb0d4778c5dc308720710e8f5e9995911.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-50318d6d13fb931307f2115b33d277b5298c45ed798719e792e722bd30615a95.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-54e3755d63a676a7fa70da77321e618032f0130c73eb6b0ae985f07c5c6c448b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-58b8b566779518776b13cabdb3498ac55b426b0493e769ea607618db7e246a71.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-59eb39577b1b566ef0a95f74f75904a62689388c0b28b77a02946952e6aa8b36.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-5df7eb0451abdb0810c1329807dab9213313858eceaea2015c5e6d584d2f168d.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-60fe557b143af00c5ddce77735e8b629dcce80126672a6c1d6bb9d2677976c75.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-63ca13cc79c27b5267ebbf53491b657ca7671e1ab9d1a4691372693a9f9a4ff8.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-679d0dd86957b91b759135cf9fc7314630cfd377b004a8c1dfedbf5764137ebd.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-6c022c7b1e5e5faf6f388528436453738aa66c36c46313ca18c7570b406553f6.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-714f90b277b3e53e84688fbe6b573cd622f5c320c33ab43c3c952aa9c12199b3.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-74132c6b056f693ac364bd5d70a40d81ccd384ffc21881d7c162895cb7d11647.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-7617f808df94ffe77c8195a911ee6b230a21fe9935e099d1310d244248ca253a.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-79d3bc72e6f424765706f7deea426379ae49cbd70dd122daedd95343618b9ac6.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-7ee9ba5adfac304c7ac930943538ecca7428b1054d451fb2f8973cf9c11caf45.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-82301526394304e59946caa3c724c377fa46ad214834bc1a876d39ad29ad92ac.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-83dea9ffef9207e919ec708a6570c6f4090dbdbbb9f5f609edbad91becbc1bb4.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-8519d142dcd5c9c6b60b9c61c24eca0b0c860eee7227124be31b2fcbff39e2bd.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-87e0c17146266f40e2f76a27c0e60bd057a7efe02fe027f9514cc8fbdba2159b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-88cb2c1126909dabe0cc1846ca2733ab98724d4b89d409fa770209d4d7627654.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-89004da6bcdd66bccd56ed64402d82dc4fb489cccaf5ca647c5a9403fa760ce9.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-8a219d75e22ca3b14ed3bfc1fbf218e2e69f540f69d0ab223170a52ac92af43c.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-8b647d9753ba2b63ba9cc0057e0b71c87592c3e3983205d6969e0b3f1dbdb74b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-922f058d237b9ffe46e13677ce2e6c4f4b39b7ae7b8b4dd248f54714d842a08e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-96d853b887f2e6d7aac6b15ddd2b71a8d8900d2ef30047da7ca1a5940503b6d0.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-9afc6f436aedd226ed0d6bf85de64c5ff57853c93ff00f768b49bf79f198590b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-9f5c0108230743e209902d457637931a2cbc77192ce513473a4f5e3498b8533c.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-a2c42d1ef0b78555844e0feddc14956117973fadd5565af9a2c48c7d2230e05f.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-a774adbda891d81dd017de5b0b50f41a5a1da6e4c5a81e51eb3cb267f10ef1fc.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b0c4750b13bbd3de768ac09f971094ac7f53c161f7054083964f472fa01dd83d.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b0e8935f157a7ae307fb050b39d0dde4e9e2e39f7cb20c716fbbcc1ffe320cd1.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b1c4d610f2fe772e88b8e78bc872f01c886031d866616effed3c6a61a99d2413.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b4d3994adddd1a4b1c769cf01690bedbf26640fdfad218b4dca813bdcee08a5a.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b79ca4a7b148d78ddf48b5c57dea59763b17693c09564a0190ed377dd09342cf.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-b8eb30360d1232b36214b17758d8c7d897251a93c7d56b310a2fc6f026fdbea9.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-c33f85b35573916eb380c8b6069939087fbd3b34b296b546a34cdfc3e7c9eee0.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-c3f8291d5ce84fb1eb463eaa0f0aed0c3ac2ed1297f345941cc8c2bfdcadedd5.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-c4d2fed1a0fb95c20ef9b4049cc5cacfb7f0a441c630528d2cde076211ff63f9.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-c8569a737734eab039a3ca9759478b203596330442305a6f3925edf67a8b52a3.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-c8e8b950c4f8645562fb36042b549b0c1f6ec3da1b9beb4d212ddd5bc640948e.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-d42e3270642ca0b9eff6b5807f600c24fd26d50d612a66edf0738c62d15680ba.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-d501983b009e4418f1753f03db0ea12db11f18e4a89a76c87b4c8ddda431cbdb.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-d536db3ccfb32aee9a48227e2b15d2404023ea24b62c159466c95899d76e29ed.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-d9956cd2e52cda4f0e7c080058d33f2bbf35c99b29e7482f198d592ff435cf28.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-dfc54abc6dd32c35bf3d3c65288ead04d9990d5f6613bd7668dd7cba38ebe3a5.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-e4e4c79727e7a83f2dfbb1d82a00a771eb3bd08aab63d2be4a1338fbde315f62.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-e70d5acce88125ae894d9d57356559c6e59e236c9650211a52fdc90602318cc3.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-e7e9e2db72c9bf45273b0cfe11057a5dbce790572f2420457e24b0fd498c2024.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-ec772d88365b803914f51dbe270f14ab6cdd2fe57c9d76e430cfef7234e92168.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-f95b295c9ffa7b96a7907c43c0557d60af3f02fc72c3f541bc94c903034a6748.jsonis excluded by!**/.sqlx/**rust/cloud-storage/soup/.sqlx/query-fc6df4dc3c0f82de02b70975338d7455df93799b12af38d50bb85d81391861c3.jsonis excluded by!**/.sqlx/**
📒 Files selected for processing (67)
js/app/packages/app/component/Root.tsxjs/app/packages/app/component/app-sidebar/sidebar.tsxjs/app/packages/app/component/app-sidebar/soup-filter-presets.tsjs/app/packages/app/component/command/CommandMenu.tsxjs/app/packages/app/component/mobile/MobileSearch.tsxjs/app/packages/app/component/next-soup/filters/configs.tsjs/app/packages/app/component/next-soup/filters/inbox-filters.tsjs/app/packages/app/component/next-soup/filters/predicates.tsjs/app/packages/app/component/next-soup/filters/query-filters.tsjs/app/packages/app/component/next-soup/soup-view/create-soup-entity-actions.tsjs/app/packages/app/component/next-soup/soup-view/filters-bar/unified-filter-dropdown.tsxjs/app/packages/app/component/split-layout/componentRegistry.tsxjs/app/packages/app/constants/list-views.tsjs/app/packages/block-canvas/component/CanvasController.tsxjs/app/packages/channel/Attachments/attachment-utils.tsjs/app/packages/core/component/AI/hook/useEntityDropAttachment.tsjs/app/packages/core/component/EntityIcon.tsxjs/app/packages/core/component/Properties/utils/entityConversion.tsjs/app/packages/entity/src/composed/ListEntity.tsxjs/app/packages/entity/src/extractors-property/entity-key-properties.tsxjs/app/packages/entity/src/extractors/entity-icon.tsxjs/app/packages/entity/src/extractors/entity-title.tsxjs/app/packages/entity/src/types/entity.tsjs/app/packages/entity/src/utils/shared.tsjs/app/packages/macro-entity/src/queries/dss.tsjs/app/packages/macro-entity/src/queries/rename.tsjs/app/packages/queries/properties/entity.tsjs/app/packages/queries/soup/normalized-cache/operations.tsjs/app/packages/queries/soup/normalized-cache/types.tsjs/app/packages/queries/soup/recently-viewed.tsjs/app/packages/queries/soup/transform-utils.tsjs/app/packages/service-clients/service-search/openapi.jsonjs/app/packages/service-clients/service-storage/openapi.jsonrust/cloud-storage/ai_tools/Cargo.tomlrust/cloud-storage/ai_tools/src/tool_context.rsrust/cloud-storage/call/Cargo.tomlrust/cloud-storage/call/src/domain/channel_name.rsrust/cloud-storage/call/src/domain/mod.rsrust/cloud-storage/call/src/domain/models.rsrust/cloud-storage/call/src/domain/ports.rsrust/cloud-storage/call/src/domain/service.rsrust/cloud-storage/call/src/outbound/pg_call_repo.rsrust/cloud-storage/document_cognition_service/Cargo.tomlrust/cloud-storage/document_cognition_service/src/api/context/test.rsrust/cloud-storage/document_cognition_service/src/main.rsrust/cloud-storage/document_storage_service/src/api/context.rsrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/document_storage_service/src/main.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/item_filters/src/ast/call.rsrust/cloud-storage/item_filters/src/lib.rsrust/cloud-storage/mcp_service/Cargo.tomlrust/cloud-storage/mcp_service/src/context.rsrust/cloud-storage/memory/Cargo.tomlrust/cloud-storage/memory/src/context.rsrust/cloud-storage/models_soup/Cargo.tomlrust/cloud-storage/models_soup/src/call_record.rsrust/cloud-storage/models_soup/src/item.rsrust/cloud-storage/models_soup/src/lib.rsrust/cloud-storage/soup/Cargo.tomlrust/cloud-storage/soup/examples/ai_tools_cli.rsrust/cloud-storage/soup/src/domain/models.rsrust/cloud-storage/soup/src/domain/service.rsrust/cloud-storage/soup/src/domain/service/tests.rsrust/cloud-storage/soup/src/inbound/toolset/list_entities.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
rust/cloud-storage/call/src/domain/service.rs (1)
688-695:⚠️ Potential issue | 🟠 MajorThread the full
Querythrough instead of stripping it tofilter().
GetCallRecordsRequestnow carries cursor/sort state, but this implementation only forwardsreq.limitplusreq.query.filter(). Cursor ids/values and non-default sort methods still never reach the repository, so paginated call results keep behaving like an unpaged first-page fetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/call/src/domain/service.rs` around lines 688 - 695, The get_user_call_records implementation drops pagination and sort state by calling req.query.filter(); instead forward the entire Query so cursor/sort propagate: update the call to repo.get_call_records_by_user to accept the full req.query (or adjust its signature if needed) and pass req.query (or a cloned Query) along with req.user_id.copied() and req.limit from the GetCallRecordsRequest in get_user_call_records so cursor ids/values and non-default sorts reach the repository.rust/cloud-storage/call/src/domain/channel_name.rs (1)
28-39:⚠️ Potential issue | 🟡 MinorNormalize blank names before returning them.
Whitespace-only
channel_namevalues, an empty participant list, and blank first/last names still collapse to""here, so the UI can render a blank call title instead of the intended fallback label.Also applies to: 48-51, 61-72, 113-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/cloud-storage/call/src/domain/channel_name.rs` around lines 28 - 39, Trim and treat whitespace-only names as empty before returning any channel title: whenever the code uses channel_name.map(|n| n.to_string()).unwrap_or_else(...) (the match arms for "organization" | "public" and the other similar arms at the noted ranges) first normalize channel_name by mapping through trim and rejecting empty/whitespace-only strings, and for participant-based fallbacks also treat participants empty or participants' first_name/last_name that are whitespace-only as missing; replace those cases with the appropriate fallback label ("Organization", "Public", or the existing fallback logic) so that blank/whitespace-only inputs don't produce an empty title.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/app-sidebar/sidebar.tsx`:
- Around line 352-358: CALLS_LINK currently uses the static PhoneCallIcon while
other sidebar items use animated icons that accept triggerAnimation on hover;
replace PhoneCallIcon with an animated variant (e.g., AnimatedPhoneCallIcon) or
create one mirroring the pattern of AnimatedInboxIcon/AnimatedChannelIcon that
accepts the triggerAnimation prop, then update CALLS_LINK.icon to that animated
component and ensure the sidebar hover logic passes triggerAnimation so the
Calls item animates consistently with other icons.
In `@js/app/packages/block-call/component/CallBlockAdapter.tsx`:
- Around line 35-40: The Show uses a truthy guard so durationMs = 0 is hidden;
change the guard to a nullish check and stop relying on the render-prop value
(which will be boolean if you use a nullish check). Update the Show invocation
from when={props.record.durationMs} to when={props.record.durationMs != null}
(or equivalent nullish check) and inside the child call
formatCallDuration(props.record.durationMs) directly instead of using the ms()
render-prop value so zero durations render correctly; reference Show,
props.record.durationMs, and formatCallDuration.
In `@js/app/packages/entity/src/composed/ListEntity.tsx`:
- Around line 556-560: The Show wrappers around duration rendering in
ListEntity.tsx use a falsy check (when={entity().durationMs}) which treats 0 as
absent; change those checks to explicitly test for null/undefined (e.g.
when={entity().durationMs != null} or when={typeof entity().durationMs ===
'number'}) so a 0-duration is rendered via formatCallDuration(ms()) instead of
falling back; update both occurrences that wrap formatCallDuration and the
related fallback logic to use the explicit null/undefined check.
In `@js/app/packages/macro-entity/src/queries/dss.ts`:
- Around line 21-24: The current isDeletable check (isDeletable(entity)) is
filtering out non-deletable types silently, causing BulkDeleteView to treat the
operation as fully successful even when items were skipped; change the
bulk-delete flow that uses isDeletable (references: isDeletable in dss.ts and
the BulkDeleteView flow) to first partition the selection into deletable and
skipped lists, perform deletes only on the deletable list, and return structured
metadata {deleted: [...], skipped: [...]} (or equivalent) to the caller instead
of a single success boolean so the UI can show an accurate completion summary;
update callers (e.g., BulkDeleteView) to surface skipped items and not close the
flow as a full success when skipped.length > 0.
- Around line 27-32: The current Promise.all on deletable.map treats false
returns from deleteItem as success so onError never fires; change the mapping to
an async function that awaits deleteItem({ id: e.id, itemType: e.type as
ItemType }), and if the awaited result is falsy/false throw a descriptive Error
(include e.id/type) so Promise.all will reject and trigger the optimistic
rollback in onError; reference the deletable variable, the deleteItem call, and
the surrounding onMutate/onError optimistic flow when making this change.
In `@rust/cloud-storage/call/src/outbound/pg_call_repo.rs`:
- Around line 704-707: The WHERE EXISTS subquery in the recent-calls query
excludes rows with cp.left_at IS NULL, which hides calls the user previously
left; update the query in pg_call_repo.rs (the recent calls method—look for the
EXISTS ... call_participants cp predicate) to remove the "AND cp.left_at IS
NULL" condition so the EXISTS checks only cp.call_id = c.id AND cp.user_id = $1,
thereby including calls the user was previously a participant in; keep the
soft-delete behavior of remove_participant() (it should still set left_at) and
adjust any tests that assumed the old behavior.
- Around line 25-43: The current helpers extract_channel_ids and
collect_channel_ids collapse the LiteralTree<CallLiteral> AST (Expr::And/Or/Not)
into a flat Vec<Uuid>, losing boolean structure and turning AND/NOT into OR;
instead update the code to preserve/handle the AST: either 1) implement a
translator from LiteralTree<CallLiteral>/Expr into an equivalent SQL predicate
(handling Expr::And, Expr::Or, Expr::Not and literal CallLiteral::ChannelId) and
return that SQL fragment for use in the query, or 2) change extract_channel_ids
to validate the tree and return an error for any unsupported Expr variant
(rejecting non-trivial shapes) so the caller won’t incorrectly treat collected
ids as an inclusive OR; reference the functions extract_channel_ids and
collect_channel_ids and the Expr variants to locate the change.
---
Duplicate comments:
In `@rust/cloud-storage/call/src/domain/channel_name.rs`:
- Around line 28-39: Trim and treat whitespace-only names as empty before
returning any channel title: whenever the code uses channel_name.map(|n|
n.to_string()).unwrap_or_else(...) (the match arms for "organization" | "public"
and the other similar arms at the noted ranges) first normalize channel_name by
mapping through trim and rejecting empty/whitespace-only strings, and for
participant-based fallbacks also treat participants empty or participants'
first_name/last_name that are whitespace-only as missing; replace those cases
with the appropriate fallback label ("Organization", "Public", or the existing
fallback logic) so that blank/whitespace-only inputs don't produce an empty
title.
In `@rust/cloud-storage/call/src/domain/service.rs`:
- Around line 688-695: The get_user_call_records implementation drops pagination
and sort state by calling req.query.filter(); instead forward the entire Query
so cursor/sort propagate: update the call to repo.get_call_records_by_user to
accept the full req.query (or adjust its signature if needed) and pass req.query
(or a cloned Query) along with req.user_id.copied() and req.limit from the
GetCallRecordsRequest in get_user_call_records so cursor ids/values and
non-default sorts reach the repository.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78b2c6d2-7c2c-40a7-a81a-586968829a3a
⛔ Files ignored due to path filters (3)
rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lockrust/cloud-storage/call/.sqlx/query-80ec8203f3090c1d40c0c782808e4a2bfbc92a06c2c37ab8a8e243f54eb93d03.jsonis excluded by!**/.sqlx/**rust/cloud-storage/call/.sqlx/query-f4f6aabb3ebbf6f8d2361b1cf8a63948bd3b0becdfdd21d780a08ffa05b2b963.jsonis excluded by!**/.sqlx/**
📒 Files selected for processing (16)
js/app/packages/app/component/app-sidebar/sidebar.tsxjs/app/packages/app/component/next-soup/filters/query-filters.tsjs/app/packages/block-call/component/CallBlockAdapter.tsxjs/app/packages/block-call/utils.tsjs/app/packages/core/constant/allBlocks.tsjs/app/packages/entity/src/composed/ListEntity.tsxjs/app/packages/macro-entity/src/queries/dss.tsjs/app/packages/queries/soup/recently-viewed.tsrust/cloud-storage/call/Cargo.tomlrust/cloud-storage/call/src/domain/channel_name.rsrust/cloud-storage/call/src/domain/models.rsrust/cloud-storage/call/src/domain/ports.rsrust/cloud-storage/call/src/domain/service.rsrust/cloud-storage/call/src/outbound/pg_call_repo.rsrust/cloud-storage/document_storage_service/src/api/swagger.rsrust/cloud-storage/soup/src/domain/models.rs
Comment on lines
+352
to
+358
| const CALLS_LINK: SidebarItem = { | ||
| id: 'calls', | ||
| label: 'Calls', | ||
| href: LIST_VIEW_PATHS.calls, | ||
| icon: PhoneCallIcon, | ||
| hotkey: 'l', | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using an animated icon for consistency.
PhoneCallIcon is a static SVG, while all other sidebar links use animated icons (e.g., AnimatedInboxIcon, AnimatedChannelIcon) that respond to the triggerAnimation prop on hover. The calls link will work correctly, but it won't have the hover animation that other sidebar items display.
♻️ Suggested approach
If an AnimatedPhoneCallIcon exists or can be created:
-import PhoneCallIcon from '@icon/duotone/phone-call-duotone.svg';
+import { AnimatedPhoneCallIcon } from '@macro-icons/wide/animating/phoneCall'; const CALLS_LINK: SidebarItem = {
id: 'calls',
label: 'Calls',
href: LIST_VIEW_PATHS.calls,
- icon: PhoneCallIcon,
+ icon: AnimatedPhoneCallIcon,
hotkey: 'l',
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/app-sidebar/sidebar.tsx` around lines 352 -
358, CALLS_LINK currently uses the static PhoneCallIcon while other sidebar
items use animated icons that accept triggerAnimation on hover; replace
PhoneCallIcon with an animated variant (e.g., AnimatedPhoneCallIcon) or create
one mirroring the pattern of AnimatedInboxIcon/AnimatedChannelIcon that accepts
the triggerAnimation prop, then update CALLS_LINK.icon to that animated
component and ensure the sidebar hover logic passes triggerAnimation so the
Calls item animates consistently with other icons.
Comment on lines
+35
to
+40
| <Show when={props.record.durationMs}> | ||
| {(ms) => ( | ||
| <> | ||
| <span>·</span> | ||
| <span>{formatCallDuration(ms())}</span> | ||
| </> |
Contributor
There was a problem hiding this comment.
Handle 0 duration explicitly in header.
Line 35 uses a truthy guard, so durationMs = 0 won’t render and shows an incorrect state. Use a nullish check instead.
Proposed fix
- <Show when={props.record.durationMs}>
+ <Show when={props.record.durationMs != null}>
{(ms) => (
<>
<span>·</span>
<span>{formatCallDuration(ms())}</span>
</>
)}
</Show>📝 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.
Suggested change
| <Show when={props.record.durationMs}> | |
| {(ms) => ( | |
| <> | |
| <span>·</span> | |
| <span>{formatCallDuration(ms())}</span> | |
| </> | |
| <Show when={props.record.durationMs != null}> | |
| {(ms) => ( | |
| <> | |
| <span>·</span> | |
| <span>{formatCallDuration(ms())}</span> | |
| </> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/block-call/component/CallBlockAdapter.tsx` around lines 35 -
40, The Show uses a truthy guard so durationMs = 0 is hidden; change the guard
to a nullish check and stop relying on the render-prop value (which will be
boolean if you use a nullish check). Update the Show invocation from
when={props.record.durationMs} to when={props.record.durationMs != null} (or
equivalent nullish check) and inside the child call
formatCallDuration(props.record.durationMs) directly instead of using the ms()
render-prop value so zero durations render correctly; reference Show,
props.record.durationMs, and formatCallDuration.
Comment on lines
+556
to
+560
| <Show | ||
| when={entity().durationMs} | ||
| fallback={entity().isActive ? 'In progress' : 'No duration'} | ||
| > | ||
| {(ms) => formatCallDuration(ms())} |
Contributor
There was a problem hiding this comment.
durationMs truthy checks hide valid 0-second calls.
Lines 557 and 703 treat 0 as absent, so ended calls with durationMs = 0 render fallback text instead of 0s.
Proposed fix
- <Show
- when={entity().durationMs}
+ <Show
+ when={entity().durationMs != null}
fallback={entity().isActive ? 'In progress' : 'No duration'}
>
{(ms) => formatCallDuration(ms())}
</Show>- <Show
- when={entity().durationMs}
+ <Show
+ when={entity().durationMs != null}
fallback={entity().isActive ? 'In progress' : ''}
>
{(ms) => formatCallDuration(ms())}
</Show>Also applies to: 702-706
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/entity/src/composed/ListEntity.tsx` around lines 556 - 560,
The Show wrappers around duration rendering in ListEntity.tsx use a falsy check
(when={entity().durationMs}) which treats 0 as absent; change those checks to
explicitly test for null/undefined (e.g. when={entity().durationMs != null} or
when={typeof entity().durationMs === 'number'}) so a 0-duration is rendered via
formatCallDuration(ms()) instead of falling back; update both occurrences that
wrap formatCallDuration and the related fallback logic to use the explicit
null/undefined check.
Comment on lines
+21
to
24
| const isDeletable = (entity: EntityData) => { | ||
| const type = entity.type; | ||
| return type !== 'chat' && type !== 'document' && type !== 'project'; | ||
| return type === 'chat' || type === 'document' || type === 'project'; | ||
| }; |
Contributor
There was a problem hiding this comment.
Don’t silently skip selected entities in bulk delete without surfacing it.
Line 21-24 filters to only chat/document/project, so mixed selections resolve “success” even when some selected items are skipped. In BulkDeleteView (Line 8-22 of that file), success immediately closes the flow, which can mismatch user expectations vs. what was actually deleted.
Suggested direction
mutationFn: async (entities: EntityData[]) => {
const deletable = entities.filter(isDeletable);
+ const skippedCount = entities.length - deletable.length;
+ if (skippedCount > 0) {
+ toast.info(`${skippedCount} selected item(s) were skipped (not deletable yet).`);
+ }
const results = await Promise.all(
deletable.map((e) => {
return deleteItem({ id: e.id, itemType: e.type as ItemType });
})
);And return deleted/skipped metadata so the caller can show an accurate completion summary.
Also applies to: 27-27, 35-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/macro-entity/src/queries/dss.ts` around lines 21 - 24, The
current isDeletable check (isDeletable(entity)) is filtering out non-deletable
types silently, causing BulkDeleteView to treat the operation as fully
successful even when items were skipped; change the bulk-delete flow that uses
isDeletable (references: isDeletable in dss.ts and the BulkDeleteView flow) to
first partition the selection into deletable and skipped lists, perform deletes
only on the deletable list, and return structured metadata {deleted: [...],
skipped: [...]} (or equivalent) to the caller instead of a single success
boolean so the UI can show an accurate completion summary; update callers (e.g.,
BulkDeleteView) to surface skipped items and not close the flow as a full
success when skipped.length > 0.
Comment on lines
+27
to
32
| const deletable = entities.filter(isDeletable); | ||
| return await Promise.all( | ||
| entities | ||
| .filter((e) => e.type !== 'channel_message') | ||
| .map((e) => { | ||
| return deleteItem({ id: e.id, itemType: e.type }); | ||
| }) | ||
| deletable.map((e) => { | ||
| return deleteItem({ id: e.id, itemType: e.type as ItemType }); | ||
| }) | ||
| ); |
Contributor
There was a problem hiding this comment.
Throw on deleteItem failures so optimistic rollback can run.
At Line 28, Promise.all resolves to booleans, and false values are currently treated as success. Since onMutate removed items optimistically, failed deletions can leave cache/UI inconsistent because onError never fires.
Proposed fix
mutationFn: async (entities: EntityData[]) => {
const deletable = entities.filter(isDeletable);
- return await Promise.all(
+ const results = await Promise.all(
deletable.map((e) => {
return deleteItem({ id: e.id, itemType: e.type as ItemType });
})
);
+ if (results.some((ok) => !ok)) {
+ throw new Error(`One or more DSS items failed to delete`);
+ }
+ return { success: true };
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/macro-entity/src/queries/dss.ts` around lines 27 - 32, The
current Promise.all on deletable.map treats false returns from deleteItem as
success so onError never fires; change the mapping to an async function that
awaits deleteItem({ id: e.id, itemType: e.type as ItemType }), and if the
awaited result is falsy/false throw a descriptive Error (include e.id/type) so
Promise.all will reject and trigger the optimistic rollback in onError;
reference the deletable variable, the deleteItem call, and the surrounding
onMutate/onError optimistic flow when making this change.
Comment on lines
+25
to
+43
| /// Extract channel_id UUIDs from a call filter AST. | ||
| fn extract_channel_ids(filter: &LiteralTree<CallLiteral>) -> Vec<Uuid> { | ||
| let Some(expr) = filter else { | ||
| return Vec::new(); | ||
| }; | ||
| let mut ids = Vec::new(); | ||
| collect_channel_ids(expr, &mut ids); | ||
| ids | ||
| } | ||
|
|
||
| fn collect_channel_ids(expr: &Expr<CallLiteral>, ids: &mut Vec<Uuid>) { | ||
| match expr { | ||
| Expr::Literal(CallLiteral::ChannelId(id)) => ids.push(*id), | ||
| Expr::And(a, b) | Expr::Or(a, b) => { | ||
| collect_channel_ids(a, ids); | ||
| collect_channel_ids(b, ids); | ||
| } | ||
| Expr::Not(inner) => collect_channel_ids(inner, ids), | ||
| } |
Contributor
There was a problem hiding this comment.
Don't collapse the filter AST into a flat channel_id = ANY(...) list.
LiteralTree<CallLiteral> can contain And, Or, and Not, but this helper drops that structure and the query later treats every collected id as an inclusive OR. NOT channel_id = X will return only X, and A AND B becomes A OR B, so non-trivial call filters return the wrong rows. Either translate the full tree to SQL or reject unsupported shapes up front.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/call/src/outbound/pg_call_repo.rs` around lines 25 - 43,
The current helpers extract_channel_ids and collect_channel_ids collapse the
LiteralTree<CallLiteral> AST (Expr::And/Or/Not) into a flat Vec<Uuid>, losing
boolean structure and turning AND/NOT into OR; instead update the code to
preserve/handle the AST: either 1) implement a translator from
LiteralTree<CallLiteral>/Expr into an equivalent SQL predicate (handling
Expr::And, Expr::Or, Expr::Not and literal CallLiteral::ChannelId) and return
that SQL fragment for use in the query, or 2) change extract_channel_ids to
validate the tree and return an error for any unsupported Expr variant
(rejecting non-trivial shapes) so the caller won’t incorrectly treat collected
ids as an inclusive OR; reference the functions extract_channel_ids and
collect_channel_ids and the Expr variants to locate the change.
Comment on lines
+704
to
+707
| WHERE EXISTS ( | ||
| SELECT 1 FROM call_participants cp | ||
| WHERE cp.call_id = c.id AND cp.user_id = $1 AND cp.left_at IS NULL | ||
| ) |
Contributor
There was a problem hiding this comment.
Don't exclude calls the user already left.
remove_participant() soft-deletes rows by setting left_at, so this predicate makes an in-progress call disappear from the recent-calls feed as soon as the user leaves, even though the method docs say it should return calls the user was a participant in.
Suggested fix
WHERE EXISTS (
SELECT 1 FROM call_participants cp
- WHERE cp.call_id = c.id AND cp.user_id = $1 AND cp.left_at IS NULL
+ WHERE cp.call_id = c.id AND cp.user_id = $1
)📝 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.
Suggested change
| WHERE EXISTS ( | |
| SELECT 1 FROM call_participants cp | |
| WHERE cp.call_id = c.id AND cp.user_id = $1 AND cp.left_at IS NULL | |
| ) | |
| WHERE EXISTS ( | |
| SELECT 1 FROM call_participants cp | |
| WHERE cp.call_id = c.id AND cp.user_id = $1 | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/call/src/outbound/pg_call_repo.rs` around lines 704 - 707,
The WHERE EXISTS subquery in the recent-calls query excludes rows with
cp.left_at IS NULL, which hides calls the user previously left; update the query
in pg_call_repo.rs (the recent calls method—look for the EXISTS ...
call_participants cp predicate) to remove the "AND cp.left_at IS NULL" condition
so the EXISTS checks only cp.call_id = c.id AND cp.user_id = $1, thereby
including calls the user was previously a participant in; keep the soft-delete
behavior of remove_participant() (it should still set left_at) and adjust any
tests that assumed the old behavior.
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.