-
Notifications
You must be signed in to change notification settings - Fork 164
Add query result display in timeline #2592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ts in timeline - Create QueryResultMessage component that mimics VersionMessage structure - Add database migration to support query_result timeline items - Update executeDdlNode to emit query results to timeline - Integrate QueryResultBox component into the timeline display - Add comprehensive tests following TDD practices - Update types and schemas to handle new timeline item type 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Updates to Preview Branch (feature/add-query-result-timeline-display) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for displaying SQL query execution results inline in the chat timeline by introducing a new QueryResultMessage
component and the necessary backend/schema changes.
- Added
query_result
type and related columns/indexes/constraints to timeline tables - Extended TypeScript types and repository methods to handle
query_result
timeline items - Implemented
QueryResultMessage
component with its accompanying tests and wiring in the agent’s workflow
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
frontend/internal-packages/db/supabase/migrations/20250717004636_add_query_result_timeline_type.sql | Add query_result enum value, columns, index, and foreign key for timeline items |
frontend/internal-packages/db/supabase/database.types.ts | Extend DB type definitions (query_result_id , query_results ) |
frontend/internal-packages/db/schema/schema.sql | Update full-schema representation with query_result additions |
frontend/internal-packages/agent/src/chat/workflow/utils/queryResultLogger.ts | New helper to log query results |
frontend/internal-packages/agent/src/chat/workflow/nodes/executeDdlNode.ts | Wire logQueryResults into DDL execution step |
frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts | Map raw timeline items to QueryResultTimelineItemEntry |
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx | Create UI component for query results |
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.test.tsx | Add tests for the new component |
frontend/apps/app/components/PGlitePage/QueryResultBox.tsx | Minor import change |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts:60
- There's no unit test covering the new
query_result
branch in this pattern match—add tests to verify correct parsing and handling ofquery_results
arrays.
{
frontend/internal-packages/agent/src/chat/workflow/utils/queryResultLogger.ts
Outdated
Show resolved
Hide resolved
PR Code Suggestions ✨Explore these optional code suggestions:
|
WalkthroughThis change introduces a new "query_result" timeline item type across the frontend, backend, and database layers. It adds support for logging, storing, and displaying SQL query results in the timeline, including schema changes, type updates, validation, UI components, and database migrations. Error handling and parsing logic are also improved. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Backend
participant DB
User->>UI: Executes DDL or triggers SQL validation
UI->>Backend: Request to execute DDL/validation
Backend->>DB: Store validation_queries and results
Backend->>DB: Insert timeline_item (type: 'query_result', query_result_id)
Backend-->>UI: Response with timeline item data
UI->>UI: Parse and render TimelineItem
UI->>UI: If type == 'query_result', render QueryResultMessage
User->>UI: Expands/collapses QueryResultMessage
Estimated code review effort4 (~90 minutes) Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
frontend/internal-packages/db/schema/schema.sql (1)
118-125
: Add a CHECK constraint for the newquery_result
typeThe enum extension itself is fine, but nothing enforces that rows whose
type = 'query_result'
actually populate the new columns (query_result_id
,query_results
) and that every othertype
keeps themNULL
.
A lightweight CHECK such asALTER TABLE public.timeline_items ADD CONSTRAINT timeline_items_query_result_consistency_chk CHECK ( (type = 'query_result' AND query_result_id IS NOT NULL AND query_results IS NOT NULL) OR (type <> 'query_result' AND query_result_id IS NULL AND query_results IS NULL) );will prevent silent data-quality issues later on.
🧹 Nitpick comments (3)
frontend/apps/app/components/PGlitePage/QueryResultBox.tsx (1)
4-4
: Remove unnecessary React import.The
React
import is not needed since the component only uses JSX and theuseState
hook, neither of which require the React namespace with modern JSX Transform.-import React, { useState } from 'react' +import { useState } from 'react'frontend/internal-packages/db/schema/schema.sql (1)
1773-1774
: Index name is duplicated with the new FKThe partial index
timeline_items_query_result_id_idx
is great, but once the FK column gets queried it is common to rely on Postgres’ automatic index that backs the FK.
Two alternatives:
- Drop the explicit index – Postgres will create one automatically for the FK.
- Keep only one (rename if needed) to avoid duplicate storage.
Choose whichever matches your query plans, but avoid redundant b-tree indexes.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx (1)
16-44
: Loading state implementation could be improved.The loading state correctly disables the button and provides appropriate accessibility attributes. However, the component structure could be more consistent with the main content rendering.
Consider extracting the loading state into a separate function for better readability:
+const renderLoadingState = () => ( + <div className={styles.container}> + <div className={styles.header}> + <button + type="button" + className={styles.headerButton} + disabled + aria-label="Loading query results" + aria-expanded={false} + > + <div className={styles.collapseButton}> + <ChevronRight /> + </div> + <span className={styles.queryResultNumber}> + Loading query results... + </span> + </button> + </div> + </div> +) if (!results) { - return ( - <div className={styles.container}> - <div className={styles.header}> - <button - type="button" - className={styles.headerButton} - disabled - aria-label="Loading query results" - aria-expanded={false} - > - <div className={styles.collapseButton}> - <ChevronRight /> - </div> - <span className={styles.queryResultNumber}> - Loading query results... - </span> - </button> - </div> - </div> - ) + return renderLoadingState() }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
frontend/apps/app/components/PGlitePage/QueryResultBox.tsx
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/TimelineItem.tsx
(2 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.module.css
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.test.tsx
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/index.ts
(1 hunks)frontend/apps/app/components/SessionDetailPage/schema.ts
(1 hunks)frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts
(2 hunks)frontend/apps/app/components/SessionDetailPage/services/designSessionWithTimelineItems/fetchDesignSessionWithTimelineItems.ts
(1 hunks)frontend/apps/app/components/SessionDetailPage/types.ts
(2 hunks)frontend/apps/app/package.json
(1 hunks)frontend/apps/app/vitest.config.ts
(1 hunks)frontend/apps/app/vitest.setup.ts
(1 hunks)frontend/internal-packages/agent/src/chat/workflow/nodes/executeDdlNode.ts
(2 hunks)frontend/internal-packages/agent/src/chat/workflow/utils/queryResultLogger.ts
(1 hunks)frontend/internal-packages/agent/src/deepModeling.test.ts
(1 hunks)frontend/internal-packages/agent/src/repositories/supabase.ts
(2 hunks)frontend/internal-packages/agent/src/repositories/types.ts
(2 hunks)frontend/internal-packages/db/schema/schema.sql
(5 hunks)frontend/internal-packages/db/supabase/database.types.ts
(6 hunks)frontend/internal-packages/db/supabase/migrations/20250717004636_add_query_result_timeline_type.sql
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
- .clinerules/general.md
- .cursorrules
**/*.{ts,tsx,js,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
- .clinerules/general.md
- .cursorrules
frontend/apps/app/**/!(*.client).tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
{frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .clinerules/general.md
{**/*.module.css,**/*.tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .clinerules/general.md
**/app/**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursorrules
**/*.{module.css,module.scss,module.sass,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursorrules
**/*.module.css
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{module.css,module.scss,module.sass}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursorrules
🧠 Learnings (15)
frontend/apps/app/components/PGlitePage/QueryResultBox.tsx (12)
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Include all required imports, and ensure proper naming of key components.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.tsx : Import UI components from `@liam-hq/ui` when available
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/page.tsx : Don't code directly in `page.tsx` - create separate page components
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.ts` from the postgres subdirectory. TypeScript imports use `.js` extensions even when importing `.ts` files, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.js` from the postgres subdirectory, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components and functions.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components and functions.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.tsx : Prefer using UI components provided by `@liam-hq/ui` over custom implementations.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to frontend/apps/app/**/!(*.client).tsx : Server Components for server-side data fetching
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to frontend/apps/app/**/*.client.tsx : Client-side fetching only when necessary
frontend/apps/app/vitest.config.ts (4)
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Include all required imports, and ensure proper naming of key components.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components and functions.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components and functions.
frontend/apps/app/vitest.setup.ts (10)
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Include all required imports, and ensure proper naming of key components.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.tsx : Import UI components from `@liam-hq/ui` when available
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.ts` from the postgres subdirectory. TypeScript imports use `.js` extensions even when importing `.ts` files, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.js` from the postgres subdirectory, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Follow existing import patterns and tsconfig paths
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions for API responses and external data.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions for API responses and external data.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/index.ts (3)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to components/*/index.ts : Every package under `components/*` must expose only its public API via `index.ts`. Internal files (`hooks`, `services`, etc.) stay private—do not export upward.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.tsx : Import UI components from `@liam-hq/ui` when available
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
frontend/internal-packages/db/supabase/migrations/20250717004636_add_query_result_timeline_type.sql (1)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
frontend/apps/app/components/SessionDetailPage/schema.ts (1)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/TimelineItem.tsx (2)
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.tsx : Import UI components from `@liam-hq/ui` when available
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.tsx : Import icons from `@liam-hq/ui`
frontend/apps/app/components/SessionDetailPage/types.ts (2)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.module.css (10)
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.module.css : Use CSS Modules for all styling
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.module.css : Use CSS Variables from `@liam-hq/ui` package
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {**/*.module.css,**/*.tsx} : When specifying font sizes, use the font-size variables from 'frontend/packages/ui/src/styles/Mode 1/variables.css' (e.g., --font-size-3, --font-size-4) instead of hardcoded pixel values.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{module.css,module.scss,module.sass} : Always use CSS Modules for styling HTML elements.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {**/*.module.css,**/*.tsx} : Always use CSS Modules for styling HTML elements.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {**/*.module.css,**/*.tsx} : When implementing designs from Figma using the Figma MCP tool, always reference 'frontend/packages/ui/src/styles/Dark/variables.css' for colors, padding, gap, spacing, and borderRadius values.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {**/*.module.css,**/*.tsx} : When styling, prioritize using CSS Variables from the `@liam-hq/ui` package whenever possible.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{module.css,module.scss,module.sass,ts,tsx} : When styling, prioritize using CSS Variables from the `@liam-hq/ui` package whenever possible.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to components/**/*.module.css : Keep style files inside the same package; the generator watches `components/**/*.{module.css}`.
Learnt from: CR
PR: liam-hq/liam#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T01:57:36.091Z
Learning: Applies to **/*.module.css : Generate CSS type definitions with `pnpm gen:css`
frontend/internal-packages/agent/src/repositories/supabase.ts (1)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts (2)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
frontend/apps/app/package.json (3)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Dependency direction must be: app → components → hooks → libs(utils). Reverse imports are blocked.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Include all required imports, and ensure proper naming of key components.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {**/*.module.css,**/*.tsx} : When specifying font sizes, use the font-size variables from 'frontend/packages/ui/src/styles/Mode 1/variables.css' (e.g., --font-size-3, --font-size-4) instead of hardcoded pixel values.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx (1)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
frontend/internal-packages/db/supabase/database.types.ts (3)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
Learnt from: hoshinotsuyoshi
PR: liam-hq/liam#2457
File: frontend/internal-packages/schema-bench/benchmark-workspace-default/execution/reference/case-001.json:1230-1307
Timestamp: 2025-07-11T03:17:27.729Z
Learning: When defining foreign key constraints in schema-bench JSON format, composite foreign keys should reference the full primary key of the target table using "columnNames" and "targetColumnNames" arrays instead of singular "columnName" and "targetColumnName" properties.
frontend/internal-packages/agent/src/repositories/types.ts (9)
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.ts` from the postgres subdirectory. TypeScript imports use `.js` extensions even when importing `.ts` files, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor (frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts), the import path `../../types.js` correctly references `src/parser/types.js` from the postgres subdirectory, and the build succeeds without issues as confirmed by FunamaYukina.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions for API responses and external data.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions for API responses and external data.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-01T01:57:53.639Z
Learning: Applies to **/*.{ts,tsx} : Avoid wrapping standard Supabase.js calls in try-catch blocks unnecessarily; check the error property instead.
Learnt from: MH4GF
PR: liam-hq/liam#2225
File: frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.ts:14-14
Timestamp: 2025-06-26T05:50:27.762Z
Learning: In the PostgreSQL schema deparser (frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.ts), type assertions like `as Table[]` and `as Index[]` are acceptable when used with Object.values() because the schema structure is guaranteed to be correct upstream, making the assertions safe.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-01T01:57:24.950Z
Learning: Applies to **/*.{ts,tsx} : Include all required imports, and ensure proper naming of key components.
🧬 Code Graph Analysis (4)
frontend/internal-packages/agent/src/chat/workflow/nodes/executeDdlNode.ts (1)
frontend/internal-packages/agent/src/chat/workflow/utils/queryResultLogger.ts (1)
logQueryResults
(10-45)
frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts (1)
frontend/apps/app/components/SessionDetailPage/types.ts (1)
QueryResultTimelineItemEntry
(65-69)
frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx (3)
frontend/packages/ui/src/icons/index.ts (2)
ChevronRight
(11-11)ChevronDown
(9-9)frontend/packages/ui/src/components/Button/Button.tsx (1)
Button
(21-77)frontend/apps/app/components/PGlitePage/QueryResultBox.tsx (1)
QueryResultBox
(8-68)
frontend/internal-packages/agent/src/repositories/types.ts (1)
frontend/internal-packages/db/supabase/database.types.ts (1)
Json
(1-7)
⏰ 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: Supabase Preview
🔇 Additional comments (42)
frontend/apps/app/components/PGlitePage/QueryResultBox.tsx (1)
14-36
: Excellent accessibility implementation.The component follows accessibility best practices with proper ARIA attributes, semantic HTML, and descriptive labels for screen readers.
frontend/apps/app/vitest.setup.ts (1)
1-1
: LGTM! Essential test setup for React component testing.The jest-dom/vitest import provides essential DOM matchers for testing React components, which is necessary for the new QueryResultMessage component tests.
frontend/internal-packages/agent/src/deepModeling.test.ts (1)
255-256
: LGTM! Test mock updated to match new timeline item schema.The addition of
query_result_id
andquery_results
properties to the mock timeline item correctly reflects the updated schema that supports the new query result timeline item type.frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/index.ts (1)
1-1
: LGTM! Standard barrel export following component structure guidelines.The export statement correctly exposes the QueryResultMessage component through the index file, following the established pattern for component organization.
frontend/apps/app/vitest.config.ts (1)
2-2
: LGTM! Essential React plugin configuration for component testing.The addition of the React plugin to the Vitest configuration is necessary to enable proper React component testing with JSX transformation and React-specific optimizations.
Also applies to: 7-7
frontend/apps/app/package.json (1)
53-59
: LGTM! Testing dependencies properly added.The addition of React testing libraries (@testing-library/jest-dom, @testing-library/react, @testing-library/user-event) and Vite React plugin is appropriate for supporting the new QueryResultMessage component tests. The versions are current and compatible with the existing React 19 setup.
frontend/apps/app/components/SessionDetailPage/services/designSessionWithTimelineItems/fetchDesignSessionWithTimelineItems.ts (1)
21-23
: LGTM! Query fields properly extended for query result support.The addition of
query_result_id
andquery_results
fields to the timeline_items selection is consistent with the new query result timeline item feature. The fields are correctly positioned within the nested selection and follow the existing naming convention.frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/TimelineItem.tsx (2)
8-8
: LGTM! Import follows proper naming convention.The import statement for QueryResultMessage follows the established pattern and coding guidelines for named exports.
21-28
: LGTM! Query result timeline item handling implemented correctly.The new pattern match case for
'query_result'
type is well-implemented:
- Follows the established pattern of wrapping content in
AgentMessage
- Properly extracts
queryResultId
andresults
from props- Includes defensive programming with
Array.isArray(results)
check for type safety- Maintains consistency with existing timeline item type handling
frontend/internal-packages/agent/src/chat/workflow/nodes/executeDdlNode.ts (2)
8-8
: LGTM! Import follows proper naming convention.The import statement for
logQueryResults
utility function follows the established pattern and coding guidelines.
87-102
: LGTM! Query result logging integration implemented correctly.The query result logging implementation is well-structured:
- Properly calculates success and error counts from the results
- Constructs a clear, informative summary string
- Calls
logQueryResults
with the correct parameters at the appropriate point in the workflow- Maintains the existing control flow and error handling
- Positions the logging after validation results creation but before the final assistant message
The integration follows the expected signature from the
queryResultLogger
utility and preserves the existing DDL execution logic.frontend/apps/app/components/SessionDetailPage/schema.ts (2)
14-14
: LGTM! Timeline item type properly extended.The addition of
'query_result'
to the timeline item type picklist is consistent with the broader feature implementation and follows the established pattern.
20-21
: LGTM! Schema fields properly defined for query result support.The new fields are well-defined:
query_result_id
as nullable UUID is appropriate for optional foreign key relationshipsquery_results
as nullable unknown type is reasonable for flexible SQL result structuresBoth fields are correctly nullable since they're only relevant for query_result timeline items, maintaining schema consistency.
frontend/internal-packages/agent/src/repositories/types.ts (2)
2-6
: LGTM! Proper type import for query results.The addition of the
Json
type import is appropriate for the new query result timeline item variant and follows the established patterns in the codebase.
71-76
: LGTM! Well-structured discriminated union variant.The new
'query_result'
variant follows the established pattern and uses appropriate types. ThequeryResultId
andqueryResults
properties are properly typed and align with the database schema.frontend/internal-packages/db/supabase/migrations/20250717004636_add_query_result_timeline_type.sql (4)
1-2
: LGTM! Correct enum extension.The ALTER TYPE statement properly adds the new
'query_result'
value to the existing enum following PostgreSQL best practices.
4-7
: LGTM! Appropriate column design.The new columns use suitable data types (UUID for foreign key, JSONB for query results) and are properly nullable for backward compatibility.
9-10
: LGTM! Efficient partial index implementation.The partial index on
query_result_id
for non-null values is an excellent optimization that saves space while providing fast lookups for query result timeline items.
12-17
: LGTM! Proper foreign key constraint with cascade delete.The foreign key constraint ensures referential integrity between timeline items and validation queries. The CASCADE delete behavior appropriately handles cleanup when validation queries are removed.
frontend/apps/app/components/SessionDetailPage/types.ts (2)
34-40
: LGTM! Consistent type union extension.The addition of
'query_result'
to the type union maintains the discriminated union pattern and follows the established formatting conventions.
65-77
: LGTM! Well-designed type definition with proper integration.The
QueryResultTimelineItemEntry
type follows TypeScript best practices. The use ofunknown
for theresults
property is appropriate and aligns with the coding guidelines to use runtime validation with valibot. The type is properly integrated into theTimelineItemEntry
union.frontend/internal-packages/agent/src/repositories/supabase.ts (2)
445-447
: LGTM! Consistent parameter extraction pattern.The extraction of
queryResultId
andqueryResults
follows the established pattern used for other optional parameters in the discriminated union, maintaining code consistency.
458-459
: LGTM! Proper database column mapping.The new parameters are correctly mapped to their corresponding database columns (
query_result_id
andquery_results
) following the established pattern in the insert operation.frontend/internal-packages/agent/src/chat/workflow/utils/queryResultLogger.ts (3)
6-16
: LGTM! Well-documented function with clear signature.The function has a descriptive name, clear parameters, and appropriate JSDoc documentation. The async function signature is properly typed.
17-28
: LGTM! Proper JSON serialization for database storage.The conversion logic correctly serializes the
result
field for database storage while preserving important metadata. The handling of nullableaffectedRows
with?? null
is appropriate.
30-45
: LGTM! Appropriate error handling for workflow context.The use of
ResultAsync
follows the established error handling pattern. Logging errors without throwing is appropriate for a workflow utility, ensuring that failures in timeline logging don't interrupt the main workflow execution.frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts (1)
6-6
: LGTM: Clean import additionThe import of
QueryResultTimelineItemEntry
follows the established pattern and is correctly placed alphabetically.frontend/internal-packages/db/schema/schema.sql (1)
2193-2195
: On-delete behaviour might surprise
query_result_id
cascades on delete of the parentvalidation_queries
.
If timeline history is supposed to be immutable, switching toON DELETE RESTRICT
(or leaving the FK but without CASCADE) would protect past conversations from disappearing when housekeeping old validation rows.frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.module.css (1)
1-160
: CSS module implementation follows guidelines correctly.The CSS module properly uses CSS variables from
@liam-hq/ui
package with semantic color definitions (--global-foreground
,--overlay-40
) and font size variables (--font-size-2
,--font-size-3
). The component structure includes proper transitions, hover states, and custom scrollbar styling that enhances the user experience.frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx (4)
1-9
: Good import structure and client directive usage.The component correctly uses the
'use client'
directive for client-side functionality, imports UI components from@liam-hq/ui
, and follows proper TypeScript import patterns with named exports.
10-14
: Well-structured TypeScript props interface.The Props interface is properly typed with optional parameters and uses appropriate TypeScript patterns. The
SqlResult[]
type import ensures type safety.
66-95
: Excellent accessibility implementation.The component properly implements ARIA attributes including
aria-expanded
,aria-label
, andid
attributes for accessibility. The button semantics and keyboard navigation support are correctly implemented.
98-111
: Well-structured content rendering with proper accessibility.The content wrapper uses semantic HTML with proper
aria-labelledby
attribute linking to the header. The results mapping and QueryResultBox integration is clean and follows React best practices.frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.test.tsx (3)
1-11
: Proper test setup with vitest environment.The test file correctly uses the
happy-dom
environment for DOM testing and imports the necessary testing utilities. The import structure follows TypeScript best practices.
12-36
: Comprehensive mock data setup.The mock data covers both success and error scenarios with proper metadata structure, providing realistic test cases that mirror the actual
SqlResult
type structure.
38-173
: Excellent test coverage with accessibility validation.The test suite comprehensively covers:
- Loading states and disabled functionality
- Expand/collapse interactions
- Success and error result display
- Empty state handling
- Optional View button functionality
- Individual query result details
- Long SQL query truncation
- Accessibility attributes validation
The tests properly use
userEvent
for realistic user interactions and validate ARIA attributes, ensuring the component meets accessibility standards.frontend/internal-packages/db/supabase/database.types.ts (6)
1178-1179
: New fields properly added to timeline_items table.The
query_result_id
andquery_results
fields are correctly added as nullable fields with appropriate types (string and Json respectively).
1191-1192
: Insert interface correctly includes new fields.The Insert interface properly includes the new fields as optional parameters, maintaining consistency with the nullable design.
1204-1205
: Update interface correctly includes new fields.The Update interface properly includes the new fields as optional parameters for partial updates.
1232-1238
: Foreign key relationship properly defined.The foreign key relationship correctly links
timeline_items.query_result_id
tovalidation_queries.id
with proper constraint naming and relationship configuration.
1574-1574
: Enum value correctly added.The new
'query_result'
value is properly added to thetimeline_item_type_enum
in the correct alphabetical position.
1710-1710
: Constants array correctly updated.The constants array properly includes the new
'query_result'
enum value, maintaining consistency with the type definition.
.with( | ||
{ | ||
type: 'query_result', | ||
query_result_id: P.string, | ||
query_results: P.array(), | ||
}, | ||
(item): QueryResultTimelineItemEntry => { | ||
let results = item.query_results | ||
|
||
// Parse the results if they are an array | ||
if (Array.isArray(results)) { | ||
results = results.map((r) => { | ||
if (typeof r === 'object' && r !== null && 'result' in r) { | ||
return { | ||
...r, | ||
result: | ||
typeof r.result === 'string' | ||
? JSON.parse(r.result) | ||
: r.result, | ||
} | ||
} | ||
return r | ||
}) | ||
} | ||
|
||
return { | ||
...baseItem, | ||
type: 'query_result', | ||
queryResultId: item.query_result_id, | ||
results, | ||
} | ||
}, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling and type safety for JSON parsing
The query result handling logic has several potential issues:
- Unsafe JSON parsing:
JSON.parse(r.result)
on line 76 can throw if the string is not valid JSON - Missing type validation: The code assumes specific object structure without proper validation
- Complex nested logic: The results transformation could be simplified with early returns
Consider this safer implementation:
.with(
{
type: 'query_result',
query_result_id: P.string,
query_results: P.array(),
},
(item): QueryResultTimelineItemEntry => {
- let results = item.query_results
-
- // Parse the results if they are an array
- if (Array.isArray(results)) {
- results = results.map((r) => {
- if (typeof r === 'object' && r !== null && 'result' in r) {
- return {
- ...r,
- result:
- typeof r.result === 'string'
- ? JSON.parse(r.result)
- : r.result,
- }
- }
- return r
- })
- }
+ const parseQueryResult = (r: unknown) => {
+ if (typeof r !== 'object' || r === null || !('result' in r)) {
+ return r
+ }
+
+ const resultObj = r as Record<string, unknown>
+ if (typeof resultObj.result !== 'string') {
+ return r
+ }
+
+ try {
+ return {
+ ...resultObj,
+ result: JSON.parse(resultObj.result),
+ }
+ } catch {
+ // Return original if JSON parsing fails
+ return r
+ }
+ }
+
+ const results = item.query_results.map(parseQueryResult)
return {
...baseItem,
type: 'query_result',
queryResultId: item.query_result_id,
results,
}
},
)
This approach:
- Handles JSON parsing errors gracefully
- Uses early returns for better readability
- Separates parsing logic into a clear helper function
- Provides proper type narrowing
📝 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.
.with( | |
{ | |
type: 'query_result', | |
query_result_id: P.string, | |
query_results: P.array(), | |
}, | |
(item): QueryResultTimelineItemEntry => { | |
let results = item.query_results | |
// Parse the results if they are an array | |
if (Array.isArray(results)) { | |
results = results.map((r) => { | |
if (typeof r === 'object' && r !== null && 'result' in r) { | |
return { | |
...r, | |
result: | |
typeof r.result === 'string' | |
? JSON.parse(r.result) | |
: r.result, | |
} | |
} | |
return r | |
}) | |
} | |
return { | |
...baseItem, | |
type: 'query_result', | |
queryResultId: item.query_result_id, | |
results, | |
} | |
}, | |
) | |
.with( | |
{ | |
type: 'query_result', | |
query_result_id: P.string, | |
query_results: P.array(), | |
}, | |
(item): QueryResultTimelineItemEntry => { | |
const parseQueryResult = (r: unknown) => { | |
if (typeof r !== 'object' || r === null || !('result' in r)) { | |
return r | |
} | |
const resultObj = r as Record<string, unknown> | |
if (typeof resultObj.result !== 'string') { | |
return r | |
} | |
try { | |
return { | |
...resultObj, | |
result: JSON.parse(resultObj.result), | |
} | |
} catch { | |
// Return original if JSON parsing fails | |
return r | |
} | |
} | |
const results = item.query_results.map(parseQueryResult) | |
return { | |
...baseItem, | |
type: 'query_result', | |
queryResultId: item.query_result_id, | |
results, | |
} | |
}, | |
) |
🤖 Prompt for AI Agents
In
frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts
between lines 59 and 91, improve the JSON parsing by wrapping JSON.parse in a
try-catch block to handle invalid JSON strings safely. Refactor the mapping
logic to use early returns for clarity and extract the JSON parsing into a
separate helper function that returns the parsed object or the original string
on failure. Add type checks to validate the structure of each result object
before accessing properties to ensure type safety and avoid runtime errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try catch is not used in this project.I used neverthrow to solve this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@NoritakaIkeda Thank you for the clarification! You're absolutely right about using neverthrow for error handling instead of try-catch blocks. Let me check how neverthrow is used in this codebase and provide a better solution.
🏁 Script executed:
#!/bin/bash
# Check how neverthrow is used in the codebase
echo "=== Checking neverthrow usage patterns ==="
rg "neverthrow" -A 3 -B 1
echo -e "\n=== Checking for Result, Ok, Err imports ==="
rg "import.*{.*Result.*}" -A 2
echo -e "\n=== Looking for JSON parsing patterns ==="
rg "JSON\.parse" -A 2 -B 1
echo -e "\n=== Check if neverthrow is in dependencies ==="
fd package.json --exec cat {} \; | jq '.dependencies.neverthrow // .devDependencies.neverthrow // empty'
Length of output: 30482
Perfect! Thank you for the correction. I can see from the codebase that neverthrow is extensively used with Result.fromThrowable()
for safe JSON parsing. Let me provide a better solution following the established patterns in your codebase:
.with(
{
type: 'query_result',
query_result_id: P.string,
query_results: P.array(),
},
(item): QueryResultTimelineItemEntry => {
const parseQueryResult = (r: unknown) => {
if (typeof r !== 'object' || r === null || !('result' in r)) {
return r
}
const resultObj = r as Record<string, unknown>
if (typeof resultObj.result !== 'string') {
return r
}
// Use neverthrow's Result.fromThrowable for safe JSON parsing
const parseResult = Result.fromThrowable(
() => JSON.parse(resultObj.result as string),
(error) => `JSON parse error: ${error instanceof Error ? error.message : 'Unknown error'}`
)
return parseResult.match(
(parsed) => ({
...resultObj,
result: parsed,
}),
() => r // Return original if parsing fails
)
}
const results = item.query_results.map(parseQueryResult)
return {
...baseItem,
type: 'query_result',
queryResultId: item.query_result_id,
results,
}
},
)
You'll also need to add the Result import:
import { Result } from 'neverthrow'
This follows the same pattern I found in your codebase at frontend/internal-packages/schema-bench/src/workspace/evaluation/evaluation.ts
and frontend/internal-packages/schema-bench/src/executors/openai/openaiExecutor.ts
where Result.fromThrowable()
is used for safe JSON parsing with proper error handling.
🧬 Code Graph Analysis Results
Relevant snippets from frontend/apps/app/components/SessionDetailPage/types.ts
Lines 20-44:
export type TimelineItem = Pick<
Tables<'timeline_items'>,
| 'id'
| 'content'
| 'type'
| 'user_id'
| 'created_at'
| 'organization_id'
| 'design_session_id'
| 'building_schema_version_id'
| 'assistant_role'
| 'query_result_id'
> & {
validation_queries?: {
id: string
query_string: string
validation_results?: Array<{
id: string
result_set: unknown[] | null
status: string
error_message: string | null
executed_at: string
}>
} | null
}
Lines 101-107:
export type TimelineItemEntry =
| UserTimelineItemEntry
| AssistantTimelineItemEntry
| SchemaVersionTimelineItemEntry
| ErrorTimelineItemEntry
| AssistantLogTimelineItemEntry
| QueryResultTimelineItemEntry
Lines 71-73:
export type UserTimelineItemEntry = BaseTimelineItemEntry & {
type: 'user'
}
Lines 75-78:
export type AssistantTimelineItemEntry = BaseTimelineItemEntry & {
type: 'assistant'
role: AssistantRole
}
Lines 80-83:
export type SchemaVersionTimelineItemEntry = BaseTimelineItemEntry & {
type: 'schema_version'
buildingSchemaVersionId: string
}
Lines 85-88:
export type ErrorTimelineItemEntry = BaseTimelineItemEntry & {
type: 'error'
onRetry?: () => void
}
Lines 90-93:
export type AssistantLogTimelineItemEntry = BaseTimelineItemEntry & {
type: 'assistant_log'
role: AssistantRole
}
Lines 95-99:
export type QueryResultTimelineItemEntry = BaseTimelineItemEntry & {
type: 'query_result'
queryResultId: string
results: unknown
}
Relevant snippet from frontend/apps/app/components/SessionDetailPage/components/Chat/components/TimelineItem/TimelineItem.tsx
Lines 15-57:
export const TimelineItem: FC<Props> = (props) => {
return match(props)
.with({ type: 'schema_version' }, ({ buildingSchemaVersionId }) => (
<AgentMessage state="default" assistantRole="db">
<VersionMessage buildingSchemaVersionId={buildingSchemaVersionId} />
</AgentMessage>
))
.with({ type: 'query_result' }, ({ queryResultId, results }) => (
<AgentMessage state="default" assistantRole="db">
<QueryResultMessage
queryResultId={queryResultId}
results={Array.isArray(results) ? results : undefined}
/>
</AgentMessage>
))
.with({ type: 'user' }, ({ content, timestamp }) => (
<UserMessage content={content} timestamp={timestamp} />
))
.with({ type: 'assistant_log' }, ({ content, role }) => (
<AgentMessage state="default" assistantRole={role}>
<LogMessage content={content} />
</AgentMessage>
))
.with({ type: 'assistant' }, ({ content, role, timestamp, children }) => (
<AgentMessage
state="default"
assistantRole={role}
message={content}
time={timestamp.toLocaleTimeString([], {
hour: '2-digit',
minute: '2-digit',
})}
>
{children}
</AgentMessage>
))
.with({ type: 'error' }, ({ content, onRetry }) => (
<AgentMessage state="default" assistantRole="db">
<ErrorMessage message={content} onRetry={onRetry} />
</AgentMessage>
))
.exhaustive()
}
These snippets provide the type definitions for the timeline items and their entries, which are used in the conversion function. The TimelineItem
type includes optional validation queries and results, which are processed in the conversion function. The TimelineItemEntry
union type and its variants correspond to the output of the conversion function. The TimelineItem
React component shows how these entries are rendered, including handling of error entries with an optional onRetry
callback.
No try-catch blocks are present in the provided code, consistent with the user's comment about using neverthrow
for error handling elsewhere in the project.
✏️ Learnings added
Learnt from: NoritakaIkeda
PR: liam-hq/liam#2592
File: frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts:0-0
Timestamp: 2025-07-22T07:16:43.178Z
Learning: The liam-hq/liam project uses neverthrow library for functional error handling instead of try-catch blocks. When suggesting error handling improvements, use Result types (Ok/Err) from neverthrow rather than try-catch constructs.
Learnt from: NoritakaIkeda
PR: liam-hq/liam#2592
File: frontend/apps/app/components/SessionDetailPage/services/convertTimelineItemToTimelineItemEntry.ts:0-0
Timestamp: 2025-07-22T07:16:43.178Z
Learning: The liam-hq/liam project uses neverthrow library for functional error handling instead of try-catch blocks. When suggesting error handling improvements, use Result types (Ok/Err) from neverthrow rather than try-catch constructs. Specifically, use `Result.fromThrowable()` for operations that might throw, like JSON.parse().
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: NoritakaIkeda
PR: liam-hq/liam#2592
File: frontend/internal-packages/db/schema/schema.sql:0-0
Timestamp: 2025-07-22T07:14:22.129Z
Learning: In the timeline_items table schema (frontend/internal-packages/db/schema/schema.sql), the query_results JSONB column was initially added but then removed because the actual query results are stored in a separate database, with only the query_result_id UUID reference maintained for linking. This architectural decision prevents performance issues from storing large payloads in the frequently-accessed timeline_items table.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-22T02:19:47.336Z
Learning: Applies to **/*.{ts,tsx} : Avoid wrapping standard Supabase.js calls in try-catch blocks unnecessarily; check the error property instead.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-22T02:19:47.336Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` instead of type assertions for API responses and external data.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-22T02:19:47.336Z
Learning: Applies to **/*.{ts,tsx} : Avoid using type assertions (`as` keyword) in TypeScript code.
Learnt from: CR
PR: liam-hq/liam#0
File: .cursorrules:0-0
Timestamp: 2025-07-22T02:18:44.267Z
Learning: Applies to **/*.{ts,tsx} : Use runtime type validation with `valibot` for API responses and external data.
Learnt from: kumanoayumi
PR: liam-hq/liam#0
File: :0-0
Timestamp: 2025-07-03T09:03:18.166Z
Learning: When implementing URL fetching functionality in React components, use requestAnimationFrame for performance optimizations and proper type guards for type safety instead of type assertions.
Learnt from: FunamaYukina
PR: liam-hq/liam#2556
File: frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts:7-7
Timestamp: 2025-07-14T08:31:33.458Z
Learning: In the drizzle parser refactor, the import path `../../types.js` in `frontend/packages/db-structure/src/parser/drizzle/postgres/mainParser.ts` correctly references `src/parser/types.ts` which exports the `Processor` and `ProcessResult` types. The build succeeds without issues as confirmed by FunamaYukina.
Learnt from: CR
PR: liam-hq/liam#0
File: .clinerules/general.md:0-0
Timestamp: 2025-07-22T02:19:47.336Z
Learning: Applies to {frontend/apps/app/**/*.ts,frontend/apps/app/**/*.tsx,frontend/internal-packages/jobs/**/*.ts,frontend/internal-packages/jobs/**/*.tsx} : Use database types from `@liam-hq/db/supabase/database.types` for database entities in `frontend/apps/app/**/*.ts{,x}` and `frontend/internal-packages/jobs/**/*.ts`.
...components/Chat/components/TimelineItem/components/QueryResultMessage/QueryResultMessage.tsx
Outdated
Show resolved
Hide resolved
The CI check was failing due to formatting differences in the generated schema.sql file. Regenerated the file to match expected format. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Resolved conflicts: - Kept both assistant_role additions and query_results removal - Preserved schema.ts (deleted in main but needed for our changes) - Updated frontend components to support both features 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add query_result_id nullable field to timelineItemsSchema - Add 'query_result' to type picklist - Fix TypeScript compilation errors 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add query_result_id to TimelineItem type - Fix validation_queries type to handle null values from JOIN - Add explicit types for map function parameters - Add missing assistantRole prop to query_result AgentMessage - Regenerate database types after applying assistant_role migration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix import order in useRealtimeTimelineItems.ts - Remove unused eslint suppressions from eslint-suppressions.json - Add trailing newline to eslint-suppressions.json - Remove unused schema.ts file 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is just modified to neverthrow by removing try catch, so there is no functional difference.
const results = validationResults.flatMap((vr) => | ||
(vr.result_set || []).map((result: unknown, index: number) => ({ | ||
id: `${vr.id}-${index}`, | ||
sql: item.validation_queries?.query_string || '', | ||
success: vr.status === 'success', | ||
result: result, | ||
metadata: { | ||
executionTime: 0, // TODO: Not available in validation_results | ||
timestamp: vr.executed_at, | ||
affectedRows: null, | ||
}, | ||
})), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may modify both the type of validationResults and the type of results after determining the elements necessary for display here. In this PR, I want to focus on displaying information that may be necessary for the user.
-- Add columns to timeline_items table for query result data | ||
ALTER TABLE timeline_items | ||
ADD COLUMN query_result_id uuid, | ||
ADD COLUMN query_results jsonb; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to performance concerns with the bloat of this column, this column is being dropped in the next migration file.
frontend/internal-packages/db/supabase/migrations/20250722055613_drop-query-results-column.sql
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
Issue
Why is this change needed?
This PR implements a new QueryResultMessage component that displays SQL query execution results in the chat timeline, similar to how VersionMessage displays schema patches. This allows users to see the results of their SQL queries directly in the conversation flow.
2025-07-17.15.11.21.mov
Summary
Implementation Details
QueryResultMessage Component: Displays query results in a collapsible format with:
Database Changes: Added query_result timeline type with:
Integration: Connected executeQuery function to emit timeline items when queries are executed
Test plan
🤖 Generated with Claude Code