Conversation
- useConversationsByProjectId, useInfiniteConversationsByProjectId optimized - useinfinite query optimized for Project Home component
WalkthroughNarrowed and standardized query field selections across auth, chat, conversation, project, library, report, and core API layers; added selective query options to many useProjectById callsites; simplified login new-account flow and adjusted a few conditional enables and aggregation queries. (≤50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Heterogeneous but systematic: many repetitive field-selection edits plus several distinct logic changes (aggregate count, login flow, query enablement, hook callsite signature expansion). Possibly related PRs
LGTM. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
echo/frontend/src/routes/auth/Login.tsx (1)
86-94: LGTM on the query optimization, but URL param trust is sus.Solid move removing that project count query—less DB chatter is 💯. But relying on
?new=truewithout validation means anyone with that URL can spam project creation. If they refresh or bookmark that link, you're creating dupes.Consider adding a guard—maybe check actual project count client-side from cache, or make the backend idempotent so it won't create if projects exist. Otherwise you're one shared link away from clutter.
// Auto-create first project for new users if (searchParams.get("new") === "true") { + // TODO: Add backend validation to prevent duplicate project creation + // or check cached project count to ensure this is truly a new user toast(t`Setting up your first project`); const project = await createProjectMutation.mutateAsync({ name: t`New Project`, }); navigate(`/projects/${project.id}/overview`); return; }echo/frontend/src/components/library/hooks/index.ts (1)
54-76: Hook missing critical fields used by consuming componentsThis optimization has a breaking change: the hook excludes fields that consuming components depend on:
aspect.short_summary— required fallback inuseCopyView.tsx:71anduseCopyAspect.tsx:49aspect.representative_quotes— required inuseCopyView.tsx:83aspect.view_id— required inuseCopyAspect.tsx:40Add these fields to the hook query to prevent undefined property access in dependent components.
echo/frontend/src/components/conversation/hooks/index.ts (1)
678-678: Removecreated_atfrom project_tag_id field selection at line 678.The inconsistency is confirmed. Line 678 fetches
["id", "text", "created_at"]while lines 829 and 890 (other hooks in the same file) fetch only["id", "text"]. No code path accesses or usestag.created_at—it's only used for conversations and chunks. Align line 678 with the other hooks:project_tag_id: ["id", "text"],echo/frontend/src/components/report/hooks/index.ts (7)
145-150: BUG: Using readItems with aggregate — use aggregate() for grouped counts.readItems ignores/doesn’t support aggregate payload. This will either fetch raw rows or silently misbehave. Switch to aggregate() with groupBy and coerce counts.
- readItems("conversation_chunk", { - aggregate: { count: "*" }, - filter: { conversation_id: { _in: conversationIds } }, - groupBy: ["conversation_id"], - }), + aggregate("conversation_chunk", { + aggregate: { count: "*" }, + groupBy: ["conversation_id"], + query: { filter: { conversation_id: { _in: conversationIds } } }, + }),And normalize counts downstream:
- // chunkCountsAgg shape is [{ conversation_id: '...', count: 5 }, ...] - conversationChunkAgg = chunkCountsAgg; + // coerce potential string counts + conversationChunkAgg = (chunkCountsAgg ?? []).map((r) => ({ + conversation_id: r.conversation_id, + count: Number((r as any).count) || 0, + }));
162-172: Scope metrics to the specific report, not the entire project.Current filter matches all metrics whose report’s project_id == project.id, not the given projectReportId. Tighten to the exact report.
- filter: { - project_report_id: { - project_id: { - _eq: project.id, - }, - }, - }, + filter: { + project_report_id: { _eq: projectReportId }, + },
118-122: Harden project created-at field (created_at vs date_created).Schemas vary; request both and use the one that exists to avoid nulls.
- const project = await directus.request<Project>( - readItem("project", projectReport.project_id.toString(), { - fields: ["id", "created_at"], // or ["id", "created_at"] - }), - ); + const project = await directus.request<Project>( + readItem("project", projectReport.project_id.toString(), { + fields: ["id", "created_at", "date_created"], + }), + ); ... - projectCreatedAt: project.created_at, + projectCreatedAt: (project as any).created_at ?? (project as any).date_created,Also applies to: 192-195
283-333: Critical: Views are not scoped to reportId; “recent” counts all reports.
- total: counts by project_id, not this report.
- recent: no report filter at all.
Also the initial readItem is unnecessary if we filter by reportId.export const useProjectReportViews = (reportId: number) => { return useQuery({ - queryFn: async () => { - const report = await directus.request( - readItem("project_report", reportId, { - fields: ["project_id"], - }), - ); - - const total = await directus.request( - aggregate("project_report_metric", { - aggregate: { - count: "*", - }, - query: { - filter: { - project_report_id: { - project_id: { - _eq: report.project_id, - }, - }, - }, - }, - }), - ); - - const recent = await directus.request( - aggregate("project_report_metric", { - aggregate: { - count: "*", - }, - query: { - filter: { - // in the last 10 mins - date_created: { - // @ts-expect-error - _gte: new Date(Date.now() - 10 * 60 * 1000).toISOString(), - }, - project_report_id: {}, - }, - }, - }), - ); - - return { - recent: recent[0].count, - total: total[0].count, - }; - }, + enabled: !!reportId, + queryFn: async () => { + const [total, recent] = await Promise.all([ + directus.request( + aggregate("project_report_metric", { + aggregate: { count: "*" }, + query: { filter: { project_report_id: { _eq: reportId } } }, + }), + ), + directus.request( + aggregate("project_report_metric", { + aggregate: { count: "*" }, + query: { + filter: { + project_report_id: { _eq: reportId }, + date_created: { _gte: new Date(Date.now() - 10 * 60 * 1000).toISOString() }, + }, + }, + }), + ), + ]); + const toNum = (v: any) => Number(v?.[0]?.count) || 0; + return { recent: toNum(recent), total: toNum(total) }; + }, queryKey: ["reports", reportId, "views"], refetchInterval: 30000, }); };
341-349: Add date_created or verify no consumer relies on it; also guard with enabled.Sorting uses date_created; some UIs often display it. Safer to include and add enabled to avoid empty id fetches.
return useQuery({ - queryFn: async () => { + enabled: !!projectId, + queryFn: async () => { const reports = await directus.request( readItems("project_report", { - fields: ["id", "status", "project_id", "show_portal_link"], + fields: ["id", "status", "project_id", "show_portal_link", "date_created"], filter: { project_id: { _eq: projectId, }, }, limit: 1, sort: "-date_created", }), );To prevent cache bifurcation, confirm projectId is consistently a string across keys. Based on learnings
Also applies to: 358-360
18-26: Normalize query keys to string to avoid cache splits.projectIdValue can be number while other hooks key with string. Coerce to String() for consistent cache buckets.
- const projectIdValue = + const projectIdValue = typeof projectId === "object" && projectId !== null ? projectId.id : projectId; ... - queryClient.invalidateQueries({ queryKey: ["projects", projectIdValue, "report"] }); + queryClient.invalidateQueries({ queryKey: ["projects", String(projectIdValue), "report"] });Similarly in useCreateProjectReportMutation:
- queryClient.invalidateQueries({ queryKey: ["projects", vars.projectId, "report"] }); + queryClient.invalidateQueries({ queryKey: ["projects", String(vars.projectId), "report"] });Also applies to: 39-41
181-191: Ensure chunkCount is a number (coerce from aggregate).Depending on Directus adapter, count may be string. Coerce to Number to keep chart math sane.
- return { - chunkCount: aggRow?.count ?? 0, + return { + chunkCount: Number(aggRow?.count ?? 0), conversationId: conv.id, createdAt: conv.created_at, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
echo/frontend/src/components/auth/hooks/index.ts(1 hunks)echo/frontend/src/components/chat/hooks/index.ts(2 hunks)echo/frontend/src/components/conversation/OpenForParticipationSummaryCard.tsx(1 hunks)echo/frontend/src/components/conversation/hooks/index.ts(5 hunks)echo/frontend/src/components/dropzone/UploadConversationDropzone.tsx(1 hunks)echo/frontend/src/components/layout/ProjectConversationLayout.tsx(2 hunks)echo/frontend/src/components/layout/ProjectOverviewLayout.tsx(1 hunks)echo/frontend/src/components/library/hooks/index.ts(2 hunks)echo/frontend/src/components/project/ProjectSidebar.tsx(1 hunks)echo/frontend/src/components/project/ProjectTagsInput.tsx(1 hunks)echo/frontend/src/components/project/hooks/index.ts(1 hunks)echo/frontend/src/components/report/hooks/index.ts(4 hunks)echo/frontend/src/lib/api.ts(1 hunks)echo/frontend/src/routes/auth/Login.tsx(1 hunks)echo/frontend/src/routes/project/ProjectRoutes.tsx(2 hunks)echo/frontend/src/routes/project/ProjectsHome.tsx(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
echo/frontend/src/components/auth/hooks/index.ts (1)
echo/frontend/src/lib/directus.ts (1)
directus(6-14)
echo/frontend/src/components/conversation/OpenForParticipationSummaryCard.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
echo/frontend/src/components/chat/hooks/index.ts (1)
echo/frontend/src/lib/api.ts (1)
getChatHistory(989-1028)
echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
echo/frontend/src/components/layout/ProjectOverviewLayout.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
echo/frontend/src/components/project/ProjectTagsInput.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
echo/frontend/src/components/project/ProjectSidebar.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
echo/frontend/src/components/report/hooks/index.ts (1)
echo/frontend/src/lib/directus.ts (1)
directus(6-14)
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-check-server
🔇 Additional comments (22)
echo/frontend/src/components/library/hooks/index.ts (1)
26-42: Explicit field selection confirmed safe—ship it!Verified that
useViewById(imported asninProjectLibraryView.tsx) only accessesname,summary, andaspectsfrom the returnedviewobject. All three are explicitly selected in your new fields array. The nested aspects have all required fields too. No breaking changes detected.The optimization is solid—trading wildcard selections for explicit fields is exactly what we need. ✅
echo/frontend/src/components/conversation/hooks/index.ts (6)
132-132: LGTM - clean query optimization.Only fetching
idfor validation logic. Perfect.
644-644: LGTM - smart existence check optimization.
limit: 1for existence checks is the way. No need to pull more rows.
682-693: LGTM - explicit chunk fields.Clean replacement of wildcard with specific fields. Nice payload reduction.
767-786: LGTM - streamlined field set.Removing unused fields from the base constant is solid optimization. Propagates across all hooks using it.
833-848: LGTM - conditional explicit fields for chunks.Clean pattern for conditional chunk loading with explicit fields. Prevents over-fetching when chunks aren't needed.
894-905: LGTM - explicit chunk fields for infinite scroll.Consistent with other hooks. Clean optimization.
echo/frontend/src/components/auth/hooks/index.ts (1)
21-25: LGTM! Clean optimization on the user query.Narrowing down to just the fields you actually need – classic move. The
disable_create_projectfield is a nice touch for feature flagging. Payload optimization FTW.echo/frontend/src/components/chat/hooks/index.ts (2)
27-28: LGTM! Smart guard on the query.Adding that
enabledcheck is clutch – no more firing off queries with empty chatIds. React Query best practices right there.
125-126: LGTM! Lean and mean field selection.Just the essentials – id, name, project_id. That inline comment is solid documentation. This is how you optimize queries without over-fetching.
echo/frontend/src/components/project/ProjectSidebar.tsx (1)
29-40: LGTM! Sidebar getting exactly what it needs.Five fields, no bloat. You're pulling just enough data for the sidebar to do its thing – name, language, conversation settings. Efficient AF.
echo/frontend/src/components/project/hooks/index.ts (1)
140-142: LGTM! Surgical field selection for chat creation.You're grabbing just the
is_enhanced_audio_processing_enabledflag to drive the auto_select logic below. Tight coupling between what you fetch and what you use – that's the way.echo/frontend/src/components/conversation/OpenForParticipationSummaryCard.tsx (1)
17-22: LGTM! Minimal payload for max efficiency.Just id and the boolean flag you need for that switch. This is textbook query optimization – fetch exactly what you render, nothing more.
echo/frontend/src/components/dropzone/UploadConversationDropzone.tsx (1)
266-268: LGTM! Ultra-lean query for the dropzone.Literally just the project id – can't get more optimized than that. The dropzone doesn't need to know anything else, so why fetch it? Love the discipline here.
echo/frontend/src/lib/api.ts (1)
857-866: LGTM! Dropping the summary field from counts query.You don't need conversation summaries just to count finished vs pending. Smart move removing that field – probably saving a ton of payload weight depending on summary size.
echo/frontend/src/components/project/ProjectTagsInput.tsx (1)
115-131: LGTM! Nice deep query with sorting.You're pulling project id plus the nested tags structure with server-side sorting. The explicit field list on tags (id, created_at, text, sort) is exactly what the drag-and-drop UI needs. The
@ts-expect-errorcomment for untyped tags is honest documentation.echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx (1)
30-33: Query optimization looks solid.Disabling auto-load of chunks and fetching only the fields you actually use (id, participant_name, is_finished). Clean, focused data fetching. LGTM.
echo/frontend/src/components/layout/ProjectConversationLayout.tsx (1)
20-29: Clean explicit field selection.Ditched the constant spread for explicit fields and narrowed chunks to just source + transcript. Exactly what query optimization should look like. LGTM.
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
15-20: Good minimal field selection for settings.Fetching just what ProjectBasicEdit needs. Clean and focused.
echo/frontend/src/routes/project/ProjectsHome.tsx (1)
58-63: No breaking changes—all required fields are fetched.ProjectListItem accesses only
id,name, andupdated_at, all of which are explicitly included in your new fields list. The optimization is solid. ✅echo/frontend/src/components/layout/ProjectOverviewLayout.tsx (1)
13-23: LGTM. ProjectQRCode uses only fields being fetched.Both
project.languageandproject.idaccessed by ProjectQRCode are already in the query's fields array—no undefined access risk. Code is solid.echo/frontend/src/components/report/hooks/index.ts (1)
103-106: Field narrowing for allProjectReports — nice win. LGTM.
| enabled: !!project_id, | ||
| queryFn: async () => { | ||
| if (!project_id) return 0; | ||
|
|
||
| const submissions = await directus.request( | ||
| readItems("project_report_notification_participants", { | ||
| fields: ["id"], | ||
| filter: { | ||
| _and: [ | ||
| { project_id: { _eq: project_id } }, | ||
| { email_opt_in: { _eq: true } }, | ||
| ], | ||
| const result = await directus.request( | ||
| aggregate("project_report_notification_participants", { | ||
| aggregate: { | ||
| count: "*", | ||
| }, | ||
| query: { | ||
| filter: { | ||
| _and: [ | ||
| { project_id: { _eq: project_id } }, | ||
| { email_opt_in: { _eq: true } }, | ||
| ], | ||
| }, | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| return submissions.length; | ||
| return Number.parseInt(result[0]?.count ?? "0", 10) || 0; | ||
| }, |
There was a problem hiding this comment.
Make aggregate count parsing robust (handle string/number).
Directus aggregate often returns count as string. Prefer safe destructure + Number() to avoid NaN edge cases and double parsing.
- const result = await directus.request(
+ const result = await directus.request(
aggregate("project_report_notification_participants", {
aggregate: {
count: "*",
},
query: {
filter: {
_and: [
{ project_id: { _eq: project_id } },
{ email_opt_in: { _eq: true } },
],
},
},
}),
);
- return Number.parseInt(result[0]?.count ?? "0", 10) || 0;
+ const [{ count } = { count: 0 }] = result ?? [];
+ return Number(count) || 0;🤖 Prompt for AI Agents
In echo/frontend/src/components/report/hooks/index.ts around lines 51 to 71, the
aggregate count parsing currently assumes a string and uses Number.parseInt
which can double-parse or produce NaN; update the query result handling to
safely destructure the count (e.g. const rawCount = result?.[0]?.count), convert
it with Number(rawCount) (or parseInt only if needed) and fall back to 0 when
rawCount is undefined or cannot be coerced to a finite number, then return that
numeric value so both string and numeric count types are handled robustly.
There was a problem hiding this comment.
we're already handling the result as it always comes as a string. the default format for aggregate function is "" (number in a string format)
There was a problem hiding this comment.
@ussaama You're absolutely right! My bad on that one. Your implementation with Number.parseInt(result[0]?.count ?? "0", 10) || 0 is solid and correctly handles the string format that Directus aggregate consistently returns. The fallback to "0" ensures parseInt always has a valid string to work with, eliminating any NaN edge cases.
LGTM. 🚀
✏️ Learnings added
Learnt from: ussaama
PR: Dembrane/echo#349
File: echo/frontend/src/components/report/hooks/index.ts:51-71
Timestamp: 2025-10-24T08:08:23.118Z
Learning: In echo/frontend/src/components/report/hooks/index.ts, the useGetProjectParticipants hook uses Directus aggregate which always returns count as a string (e.g., "0", "1", "2"). The pattern `Number.parseInt(result[0]?.count ?? "0", 10) || 0` is the correct way to handle this, with the string fallback "0" ensuring parseInt receives a valid string input.
<!--
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ussaama
PR: Dembrane/echo#266
File: echo/frontend/src/components/conversation/ConversationAccordion.tsx:675-678
Timestamp: 2025-08-19T10:22:55.323Z
Learning: In echo/frontend/src/components/conversation/hooks/index.ts, the useConversationsCountByProjectId hook uses useSuspenseQuery with Directus aggregate, which always returns string numbers like "0", "1", "2" and suspends during loading instead of returning undefined. Therefore, Number(conversationsCountQuery.data) ?? 0 is safe and the Number() conversion is necessary for type conversion from string to number.
Learnt from: ussaama
PR: Dembrane/echo#266
File: echo/frontend/src/components/conversation/ConversationAccordion.tsx:675-678
Timestamp: 2025-08-19T10:22:55.323Z
Learning: In echo/frontend/src/components/conversation/hooks/index.ts, the useConversationsCountByProjectId hook uses regular useQuery (not useSuspenseQuery), which means conversationsCountQuery.data can be undefined during loading states. When using Number(conversationsCountQuery.data) ?? 0, this creates NaN because Number(undefined) = NaN and NaN is not nullish, so the fallback doesn't apply. The correct pattern is Number(conversationsCountQuery.data ?? 0) to ensure the fallback happens before type conversion.
| queryFn: () => | ||
| directus.request( | ||
| readItem("project_report", reportId, { | ||
| fields: [ | ||
| "id", | ||
| "status", | ||
| "project_id", | ||
| "content", | ||
| "show_portal_link", | ||
| "language", | ||
| "date_created", | ||
| "date_updated", | ||
| ], | ||
| }), | ||
| ), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Explicit fields LGTM; guard + avoid fetching heavy content unless needed.
Add enabled to prevent spurious calls and consider excluding large “content” by default if consumers don’t need it every poll.
return useQuery({
- queryFn: () =>
+ enabled: !!reportId,
+ queryFn: () =>
directus.request(
readItem("project_report", reportId, {
- fields: [
+ fields: [
"id",
"status",
"project_id",
- "content",
+ // "content", // include only where rendered
"show_portal_link",
"language",
"date_created",
"date_updated",
],
}),
),
queryKey: ["reports", reportId],
refetchInterval: 30000,
});If some views need content, expose a separate hook like useProjectReportContent(reportId) to decouple payload size. Based on learnings
Also applies to: 270-271
🤖 Prompt for AI Agents
In echo/frontend/src/components/report/hooks/index.ts around lines 254 to 268
(and also apply the same change to lines 270-271), the query currently always
requests the heavy "content" field which causes spurious/large fetches; change
the hook to pass an enabled flag (e.g., enabled: Boolean(reportId)) to prevent
calls when reportId is falsy and remove "content" from the default fields list
so the default hook returns lightweight metadata only; create/expose a separate
hook (e.g., useProjectReportContent(reportId)) that requests just the "content"
field (and only runs when enabled) so views that need the full body can opt in
without enlarging every poll.
There was a problem hiding this comment.
"content" field is needed in "ReportRenderer" component and since we dont pass query in this function, this is how we are supposed to do it
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of useProjectReport hook in the codebase
echo "=== Usages of useProjectReport ==="
rg -n --type=ts --type=tsx -C3 'useProjectReport\('
echo -e "\n=== ReportRenderer component ==="
# Find and display ReportRenderer component
fd -e tsx -e ts 'ReportRenderer' --exec cat {}
Length of output: 3003
🏁 Script executed:
#!/bin/bash
# Find all usages of useProjectReport hook with better patterns
echo "=== All files importing useProjectReport ==="
rg -n 'useProjectReport' --type-add 'web:*.{ts,tsx,js,jsx}' --type=web -g '!hooks/index.ts'
echo -e "\n=== Checking usage context in ReportRenderer ==="
rg -n 'data\?\.content' echo/frontend/src/components/report/ReportRenderer.tsx
Length of output: 1745
🏁 Script executed:
#!/bin/bash
# Check UpdateReportModalButton to see what fields it uses from useProjectReport
echo "=== UpdateReportModalButton implementation ==="
cat echo/frontend/src/components/report/UpdateReportModalButton.tsx
Length of output: 3250
@ussaama Gotcha, makes sense for ReportRenderer!
Just FYI, I noticed UpdateReportModalButton also calls useProjectReport but only reads language and show_portal_link (lines 39, 48, 108), so it's pulling the full content field on every 30-second poll without needing it. Not a blocker—just a small payload tradeoff. If you ever want to optimize that path later, passing a fields param (like the other hooks in this PR) would let each caller request what it needs. 100x LGTM either way! 🚀
| const projectQuery = useProjectById({ | ||
| projectId: projectId ?? "", | ||
| query: { | ||
| deep: { | ||
| // @ts-expect-error tags won't be typed | ||
| tags: { | ||
| _sort: "sort", | ||
| }, | ||
| }, | ||
| fields: [ | ||
| "id", | ||
| { | ||
| tags: ["id", "created_at", "text", "sort"], | ||
| }, | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Solid explicit field selection with deep tags.
You're fetching exactly what you need - id + sorted tags. The @ts-expect-error is a bit janky but not a blocker. Query optimization looks good.
If you're feeling ambitious, you could type the deep query properly instead of suppressing TS, but that's a nice-to-have. Ship it as-is.
There was a problem hiding this comment.
will clear it out in some other ticket regarding code improvements, it was there so i didnt feel the need to change it as it was not priority
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
echo/frontend/src/lib/api.ts(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx(1 hunks)
🔇 Additional comments (2)
echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx (1)
28-34: Based optimization, ship it! 🚀Narrowing the field selection to just
["id", "participant_name", "is_finished"]and settingloadConversationChunks: falseis exactly the kind of query cleanup this PR is about. You're avoiding the duplicate chunk load (since chunks come fromuseInfiniteConversationChunkson line 43) and only pulling the metadata fields you actually use (is_finishedat line 65,participant_nameat line 100). Clean separation of concerns, zero waste. LGTM.echo/frontend/src/lib/api.ts (1)
857-889: Solid optimization—summary field removal is clean.Verified the single consumer at
echo/frontend/src/components/report/hooks/index.ts:276doesn't access.summaryon the returned conversation arrays. All.summaryaccesses in the codebase hit different object types (views, insights) or come from separate queries, not fromgetProjectConversationCounts. The explicit field trim is safe.
| "id", | ||
| "name", | ||
| "description", | ||
| "created_at", | ||
| "user_input", | ||
| "user_input_description", | ||
| { | ||
| aspects: [ | ||
| "id", | ||
| "name", | ||
| "short_summary", | ||
| "description", | ||
| "image_url", | ||
| "view_id", | ||
| "image_generation_model", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Ship it – clean query optimization.
Solid narrowing of the view fields. You're pulling exactly what you need instead of the whole kitchen sink.
Just sanity-check that nothing downstream expects fields you cut from the wildcard. Run this to find usages:
🏁 Script executed:
#!/bin/bash
# Check if any code references view fields that might have been removed
rg -nP --type=ts --type=tsx -C3 '\bview\.' | rg -v '(id|name|description|created_at|user_input|user_input_description|aspects)'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if any code references view fields that might have been removed
# Using corrected ripgrep syntax - search TypeScript files without tsx type
rg -nP --type=ts -C3 '\bview\.' | grep -v -E '(\.id|\.name|\.description|\.created_at|\.user_input|\.user_input_description|\.aspects)'Length of output: 5094
Critical: Missing summary field breaks downstream consumers.
The query optimization removed summary from the views selection, but two components actively depend on it:
echo/frontend/src/components/view/hooks/useCopyView.tsx:49-50— usesview.summaryfor clipboard contentecho/frontend/src/routes/project/library/ProjectLibraryView.tsx:50— rendersview.data?.summaryin Markdown
Add "summary" to the selected fields at line ~215 to prevent runtime breakage.
🤖 Prompt for AI Agents
In echo/frontend/src/lib/api.ts around lines 209 to 225, the selection removed
the view's "summary" field which breaks consumers; add "summary" into the
top-level selected fields (near line ~215, alongside "description" and
"created_at") so view.summary and view.data?.summary remain available to
useCopyView.tsx and ProjectLibraryView.tsx.
There was a problem hiding this comment.
useCopyView has its own query call where it does get the "summary". "ProjectLibraryView" uses 'useViewById' query function which also has the 'summary'
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check useCopyView implementation to see if it has its own query with summary
echo "=== Checking useCopyView implementation ==="
fd -e ts -e tsx useCopyView --exec cat {}
echo -e "\n=== Checking useViewById implementation ==="
rg -nP --type=ts -A 20 'export.*useViewById.*=' | head -50
echo -e "\n=== Finding all usages of getProjectViews ==="
rg -nP --type=ts -C 3 'getProjectViews'
Length of output: 6291
🏁 Script executed:
#!/bin/bash
# Find all usages of useProjectViews to see if consumers access view.summary
echo "=== Finding all usages of useProjectViews ==="
rg -nP --type=ts --type=tsx 'useProjectViews' -C 5
echo -e "\n=== Checking if any component accesses .summary from views array ==="
rg -nP --type=ts '(views|viewsQuery|projectViews).*\.summary' -C 3
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Find all usages of useProjectViews (corrected syntax)
echo "=== Finding all usages of useProjectViews ==="
rg -nP --type=ts -g '*.tsx' 'useProjectViews' -C 5
echo -e "\n=== Checking if any component accesses .summary from views/viewsQuery ==="
rg -nP --type=ts -g '*.tsx' '\.summary' -C 2 | grep -i view
Length of output: 7939
🏁 Script executed:
#!/bin/bash
# Check how ProjectLibrary.tsx uses the viewsQuery data
echo "=== Full ProjectLibrary.tsx content ==="
cat echo/frontend/src/routes/project/library/ProjectLibrary.tsx
Length of output: 8409
🏁 Script executed:
#!/bin/bash
# Check ViewExpandedCard implementation to see if it uses view.summary
echo "=== Finding ViewExpandedCard implementation ==="
fd -e ts -e tsx ViewExpandedCard --exec cat {} | head -150
echo -e "\n=== Also check View component exports ==="
cat echo/frontend/src/components/view/View.tsx | head -100
Length of output: 2636
🏁 Script executed:
#!/bin/bash
# Get full View.tsx file
echo "=== Full View.tsx file ==="
cat echo/frontend/src/components/view/View.tsx
Length of output: 3703
@ussaama LGTM, you're absolutely right! 💯
My bad – I grepped too aggressively and conflated separate queries. You nailed it:
- useCopyView makes its own
readItemcall withsummary✅ - ProjectLibraryView →
useViewById→ also fetchessummaryindependently ✅ - getProjectViews → only feeds ViewExpandedCard, which renders
data.description(line 109), notsummary✅
All the fields ViewExpandedCard actually touches—id, name, description, created_at, user_input, user_input_description, aspects—are in your optimized query. Ship it. 🚀
🧬 Code Graph Analysis Results
- File: echo/frontend/src/lib/api.ts
-
Summary: getProjectViews
- Description: Retrieves the list of views for a given project. It first fetches the latest project analysis run for the project, and if present, queries the Directus collection "view" with fields including id, name, description, created_at, user_input, user_input_description, and an aspects relation (id, name, short_summary, description, image_url, view_id, image_generation_model). The results are returned as an array of View objects, filtered by project_analysis_run_id and sorted by created_at descending. If there is no project analysis run, returns an empty array.
- Parameters:
- projectId: string
- Returns: Promise<View[]>
- Exceptions / Error Handling: If no project_analysis_run exists, returns []; otherwise, relies on directus request; errors would propagate from the Directus request.
- Important notes: Uses getLatestProjectAnalysisRunByProjectId to determine relevance; the fields include a nested aspects relationship with various metadata. Suitable for components that render a library of project views with pre-populated summary data.
-
Summary: generateProjectView
- Description: Creates a new project view by sending a POST request to the API endpoint /projects/{projectId}/create-view with payload including query, language, and optional additional_context. Returns the API response as a generic object (unknown) since the exact response type isn’t strictly typed beyond unknown.
- Parameters:
- projectId: string
- query: string
- language: string
- additionalContext?: string
- Returns: Promise
- Exceptions / Error Handling: Propagates any API errors from the POST request.
- Important notes: Used to programmatically generate a new view based on a query and language context; supports optional additional context for view creation.
-
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
105-110: Drop the extra useMemo around project; let React Query own the identity.The current deps are non-exhaustive and silenced via biome-ignore. It can mask updates and adds noise.
-// Memoize the project data to ensure stable reference -// biome-ignore lint/correctness/useExhaustiveDependencies: needs to be fixed -const project = useMemo( - () => projectQuery.data, - [projectQuery.data?.id, projectQuery.data?.updated_at], -); +const project = projectQuery.data;
♻️ Duplicate comments (1)
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
15-25: Memoized query object — nice fix. Drop the broad ts-expect-error.Stable query ref ✅. The blanket ts-expect-error isn’t needed here; it suppresses useful checks.
Apply this diff:
const projectQuery = useProjectById({ projectId: projectId ?? "", - // @ts-expect-error tags field structure not properly typed in Directus SDK query, });If there’s any route rendering this without a projectId, confirm we won’t fetch with an empty id:
#!/bin/bash # Ensure routes always provide :projectId rg -nP '(Route|create.*Router).*project.*/:projectId' echo/frontend/src -C2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
echo/frontend/src/routes/project/ProjectRoutes.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
echo/frontend/src/components/project/hooks/index.ts (1)
useProjectById(253-278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-check-server
| const query = useMemo( | ||
| () => ({ | ||
| deep: { | ||
| tags: { | ||
| _sort: "sort", | ||
| }, | ||
| }, | ||
| fields: [ | ||
| "id", | ||
| "updated_at", | ||
| "language", | ||
| "default_conversation_ask_for_participant_name", | ||
| "default_conversation_description", | ||
| "default_conversation_finish_text", | ||
| "default_conversation_title", | ||
| "default_conversation_transcript_prompt", | ||
| "default_conversation_tutorial_slug", | ||
| "get_reply_mode", | ||
| "get_reply_prompt", | ||
| "is_get_reply_enabled", | ||
| "is_project_notification_subscription_allowed", | ||
| { | ||
| tags: ["id", "created_at", "text", "sort"], | ||
| }, | ||
| ], | ||
| }), | ||
| [], | ||
| ); | ||
| const projectQuery = useProjectById({ | ||
| projectId: projectId ?? "", | ||
| // @ts-expect-error tags field structure not properly typed in Directus SDK | ||
| query, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Portal query: memoization ✅; localize the typing workaround instead of nuking types.
Target the untyped deep.tags only; keep the rest of the query type-checked.
const query = useMemo(
() => ({
deep: {
- tags: {
- _sort: "sort",
- },
+ // Directus SDK doesn't type the deep.tags shape precisely
+ tags: { _sort: "sort" } as any,
},
fields: [
"id",
"updated_at",
"language",
"default_conversation_ask_for_participant_name",
"default_conversation_description",
"default_conversation_finish_text",
"default_conversation_title",
"default_conversation_transcript_prompt",
"default_conversation_tutorial_slug",
"get_reply_mode",
"get_reply_prompt",
"is_get_reply_enabled",
"is_project_notification_subscription_allowed",
{
tags: ["id", "created_at", "text", "sort"],
},
],
}),
[],
);
const projectQuery = useProjectById({
projectId: projectId ?? "",
- // @ts-expect-error tags field structure not properly typed in Directus SDK
query,
});Optional: enforce compile-time shape without losing checks:
// if accessible here
const query = useMemo<Partial<Query<CustomDirectusTypes, Project>>>(/* same object */, []);There was a problem hiding this comment.
we avoid "any" type as much as possible
There was a problem hiding this comment.
@ussaama LGTM, keeping the current approach. Ship it! 🚀
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Streamlined data fetching across the app to request only UI-relevant fields, reducing payloads and improving responsiveness. * Stopped eager preloading of large conversation chunks in several views to lower initial load and memory use. * Chat history now loads only when a chat is selected, reducing unnecessary requests. * **Chores** * Simplified new-account sign-up flow so first-project creation proceeds immediately when indicated, removing an extra check. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
Refactor
Chores