-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Google Sheets dedicated transformer system #185
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
…rag range selection
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRefactors Google Sheets metric UI from single-column to click-and-drag multi-cell range selection, changes template parameter from COLUMN_INDEX to DATA_RANGE (A1 notation), adds Google Sheets-specific AI generators/prompts for ingestion and chart code, routes generation based on templateId, and persists per-operation chart/transform errors. Changes
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant Client as GoogleSheets Component
participant Server as Metric API
participant Router as AI Code Generator Router
participant Generator as GSheets Generator
participant AI as OpenRouter API
participant DB as Database
User->>Client: Drag-select a range
Client->>Client: Build A1 notation (Sheet1!A1:D20)
Client->>Server: Create metric with templateId="gsheets-data" and DATA_RANGE
Server->>Router: Request code generation (templateId="gsheets-data", endpointConfig)
Router->>Router: Detect "gsheets-" templateId -> route to GSheets generators
Router->>Generator: generate data ingestion / chart code (include sample 2D data + endpointConfig)
Generator->>AI: Send prompt + sample data
AI-->>Generator: Return JS transform code
Generator-->>Router: Return GeneratedCode (code + rationale)
Router->>Server: Apply generated transformer / attempt chart generation
Server->>DB: Persist metric, transformer, and any chartError/lastError
DB-->>Server: Confirm persist
Server-->>Client: Respond metric created (with status/errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/server/api/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{tsx,ts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (4)
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 |
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
🧹 Nitpick comments (5)
src/server/api/routers/metric.ts (1)
138-182: Consider combining the two metric update calls into one.Lines 172-175 and 179-182 both update the same metric record. Consolidating them reduces a database round trip.
- // Store any errors for user visibility - if (chartError) { - await ctx.db.metric.update({ - where: { id: dashboardChart.metricId }, - data: { lastError: chartError }, - }); - } - - // Update lastFetchedAt + // Update metric with lastFetchedAt and any errors await ctx.db.metric.update({ where: { id: dashboardChart.metricId }, - data: { lastFetchedAt: new Date() }, + data: { + lastFetchedAt: new Date(), + ...(chartError && { lastError: chartError }), + }, });src/server/api/services/transformation/gsheets/generator.ts (2)
41-48: Consider extracting shared OpenRouter client factory.This
getOpenRouterClientfunction duplicates the one inai-code-generator.ts. Consider extracting to a shared utility in./utils.tsto avoid duplication.
186-193: Add type guard for dimension property access.Accessing
.labeland.seriesonRecord<string, unknown>works at runtime due to JavaScript's flexibility, but the type narrowing could be more explicit for maintainability.+ const getDimensionValue = (d: Record<string, unknown> | null, key: string): unknown => + d && typeof d === 'object' ? d[key] : undefined; + const hasLabels = allDimensions.some((d) => d && "label" in d); const hasSeries = allDimensions.some((d) => d && "series" in d); const uniqueLabels = [ - ...new Set(allDimensions.map((d) => d?.label).filter(Boolean)), + ...new Set(allDimensions.map((d) => getDimensionValue(d, 'label')).filter(Boolean)), ]; const uniqueSeries = [ - ...new Set(allDimensions.map((d) => d?.series).filter(Boolean)), + ...new Set(allDimensions.map((d) => getDimensionValue(d, 'series')).filter(Boolean)), ];src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx (2)
157-167: Potential performance concern with large datasets.Using
Math.max(...fullData.map(...))spreads all elements as function arguments. For very large sheets (thousands of rows), this could hit JavaScript's call stack limit or cause performance degradation.Consider using a reduce-based approach for large datasets:
// Default to entire sheet if no selection if (fullData.length > 0) { - const maxCols = Math.max(...fullData.map((row) => row.length)); + const maxCols = fullData.reduce((max, row) => Math.max(max, row.length), 0); return selectionToA1Notation( selectedSheet, 0, 0, fullData.length - 1, maxCols - 1, ); }
199-209: Consider adding defensive validation for all required parameters.The function only explicitly checks
connectionandmetricName, but relies onendpointParamsbeing populated. While the step-based flow should ensure this, explicit validation would be more defensive.const handleCreateMetric = () => { - if (!connection || !metricName) return; + if (!connection || !metricName || !spreadsheetId || !selectedSheet) return; void onSubmit({ templateId: TEMPLATE_ID,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx(7 hunks)src/lib/integrations/google-sheets.ts(2 hunks)src/server/api/routers/metric.ts(2 hunks)src/server/api/services/transformation/ai-code-generator.ts(5 hunks)src/server/api/services/transformation/chart-generator.ts(4 hunks)src/server/api/services/transformation/gsheets/generator.ts(1 hunks)src/server/api/services/transformation/gsheets/index.ts(1 hunks)src/server/api/services/transformation/gsheets/prompts.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: In TypeScript files, use@typescript-eslint/consistent-type-importsto enforce inline type imports and sort imports with @trivago/prettier-plugin-sort-imports
Use the tRPC server caller API fromsrc/trpc/server.tsdirectly in Server Components for 10x faster performance instead of client-side hooks
Files:
src/server/api/services/transformation/chart-generator.tssrc/server/api/services/transformation/gsheets/prompts.tssrc/server/api/routers/metric.tssrc/server/api/services/transformation/ai-code-generator.tssrc/server/api/services/transformation/gsheets/index.tssrc/lib/integrations/google-sheets.tssrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/server/api/services/transformation/gsheets/generator.ts
src/server/api/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Add new tRPC routers in
src/server/api/routers/[name].tsand register them in theappRouterinsrc/server/api/root.ts
Files:
src/server/api/services/transformation/chart-generator.tssrc/server/api/services/transformation/gsheets/prompts.tssrc/server/api/routers/metric.tssrc/server/api/services/transformation/ai-code-generator.tssrc/server/api/services/transformation/gsheets/index.tssrc/server/api/services/transformation/gsheets/generator.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TanStack Query (via tRPC hooks) for server state management, client-side data fetching, and automatic cache invalidation
Files:
src/server/api/services/transformation/chart-generator.tssrc/server/api/services/transformation/gsheets/prompts.tssrc/server/api/routers/metric.tssrc/server/api/services/transformation/ai-code-generator.tssrc/server/api/services/transformation/gsheets/index.tssrc/lib/integrations/google-sheets.tssrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/server/api/services/transformation/gsheets/generator.ts
src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/server/api/routers/**/*.ts: UseprotectedProcedurefor tRPC routes that require authentication instead of manually checkingctx.user. The route handler and middleware handle authentication automatically.
Use authorization helper utilities fromsrc/server/api/utils/authorization.ts(getUserOrganizationId(),getTeamAndVerifyAccess()) to enforce multi-tenant isolation
Files:
src/server/api/routers/metric.ts
src/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React Server Components and server-side data fetching patterns instead of client-side fetching for better performance and security
Files:
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-02T13:36:55.215Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T13:36:55.215Z
Learning: Applies to src/app/**/store/**/*.tsx : Store canvas state in the database (Prisma models) and use Zustand with Context pattern for client-side state management in React Flow features
Applied to files:
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx
📚 Learning: 2025-12-02T13:36:55.215Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T13:36:55.215Z
Learning: Applies to src/app/**/*.{tsx,ts} : Use React Server Components and server-side data fetching patterns instead of client-side fetching for better performance and security
Applied to files:
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx
🧬 Code graph analysis (3)
src/server/api/services/transformation/gsheets/prompts.ts (1)
src/server/api/services/transformation/gsheets/index.ts (2)
GSHEETS_DATA_INGESTION_PROMPT(12-12)GSHEETS_CHART_PROMPT(12-12)
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx (4)
src/components/ui/label.tsx (1)
Label(25-25)src/components/ui/button.tsx (1)
Button(62-62)src/components/ui/scroll-area.tsx (1)
ScrollArea(59-59)src/components/ui/table.tsx (6)
Table(108-108)TableHeader(109-109)TableRow(113-113)TableHead(112-112)TableBody(110-110)TableCell(114-114)
src/server/api/services/transformation/gsheets/generator.ts (2)
src/server/api/services/transformation/utils.ts (2)
safeStringifyForPrompt(49-86)cleanGeneratedCode(2-25)src/server/api/services/transformation/gsheets/prompts.ts (2)
GSHEETS_DATA_INGESTION_PROMPT(11-85)GSHEETS_CHART_PROMPT(87-204)
⏰ 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: test
🔇 Additional comments (13)
src/server/api/services/transformation/gsheets/prompts.ts (2)
1-85: LGTM! Well-structured data ingestion prompt.The prompt covers essential scenarios (time-series, categorical, numeric grid) with clear examples and rules. Good emphasis on unique timestamps and proper number parsing.
87-204: LGTM! Comprehensive chart prompt with good multi-series handling.The chart type selection guidance and multi-series transformation examples are well-documented. The prompt correctly specifies the shadcn/ui chart format.
src/server/api/services/transformation/gsheets/index.ts (1)
1-12: LGTM!Clean barrel export for the Google Sheets transformation module.
src/server/api/services/transformation/ai-code-generator.ts (1)
259-270: LGTM! Chart transformer routing is correct.The routing uses optional chaining appropriately and passes through the relevant input fields.
src/server/api/services/transformation/chart-generator.ts (2)
131-144: LGTM! TemplateId propagation is correctly implemented.The fallback from
input.templateIdtodashboardChart.metric.templateIdensures routing works for both creation and cases where templateId isn't explicitly passed.Minor note:
templateId ?? undefinedon line 143 is slightly redundant sincenullwould also be acceptable to the optional parameter, but it's not incorrect.
301-301: LGTM!Consistent templateId propagation in the regeneration path.
src/lib/integrations/google-sheets.ts (1)
53-99: LGTM! Template updated for range-based data selection.Good design choice:
previewEndpointfetches the entire sheet for UI preview, whilemetricEndpointuses the user-selectedDATA_RANGE. The A1 notation input aligns with Google Sheets API conventions.src/server/api/services/transformation/gsheets/generator.ts (2)
53-100: LGTM! Data ingestion generator is well-structured.Good approach extracting a preview of the first 10 rows for the prompt while including the full response structure. The reasoning output provides useful context.
169-241: LGTM! Chart generator provides rich context for AI.Good data analysis (hasLabels, hasSeries, unique values) gives the AI sufficient context to choose appropriate chart types. The prompt correctly suggests the AI should override the user's chartType suggestion if a better fit exists.
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx (4)
1-29: LGTM!Imports are well-organized and appropriate for this client component. Using tRPC hooks for the interactive dialog workflow is suitable here.
211-265: LGTM!The drag-to-select implementation is well-structured. The selection logic correctly normalizes coordinates regardless of drag direction, and the global mouseup listener is properly cleaned up.
267-275: LGTM!Validation logic is appropriate. The
maxColscalculation here operates onpreviewDatawhich is limited to 20 rows, so the spread operator usage is acceptable.
381-411: LGTM!The table rendering with drag-to-select functionality is well-implemented. The cell selection visual feedback using ring and background color provides clear user feedback, and the truncation for long cell values prevents layout issues.
| // Convert selection to A1 notation | ||
| function selectionToA1Notation( | ||
| sheetName: string, | ||
| startRow: number, | ||
| startCol: number, | ||
| endRow: number, | ||
| endCol: number, | ||
| ): string { | ||
| const startColLetter = columnToLetter(startCol); | ||
| const endColLetter = columnToLetter(endCol); | ||
| // Rows are 1-indexed in A1 notation | ||
| return `${sheetName}!${startColLetter}${startRow + 1}:${endColLetter}${endRow + 1}`; | ||
| } |
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.
Sheet names with spaces or special characters need quoting in A1 notation.
If the sheet name contains spaces, single quotes, or other special characters, the A1 notation will be invalid. Google Sheets requires such names to be wrapped in single quotes (e.g., 'Sheet Name'!A1:B10).
function selectionToA1Notation(
sheetName: string,
startRow: number,
startCol: number,
endRow: number,
endCol: number,
): string {
const startColLetter = columnToLetter(startCol);
const endColLetter = columnToLetter(endCol);
+ // Quote sheet name if it contains spaces or special characters
+ const needsQuoting = /[\s'!]/.test(sheetName);
+ const quotedSheetName = needsQuoting
+ ? `'${sheetName.replace(/'/g, "''")}'`
+ : sheetName;
// Rows are 1-indexed in A1 notation
- return `${sheetName}!${startColLetter}${startRow + 1}:${endColLetter}${endRow + 1}`;
+ return `${quotedSheetName}!${startColLetter}${startRow + 1}:${endColLetter}${endRow + 1}`;
}🤖 Prompt for AI Agents
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx around
lines 48 to 60: the function building A1 notation does not quote or escape sheet
names, so names with spaces or special chars produce invalid A1 strings; update
it to wrap the sheetName in single quotes and escape any single quotes inside
the name by doubling them (replace ' with ''), e.g. `'Sheet Name'!A1:B2`; ensure
the quoting is applied consistently (you can always quote) and only the sheet
name is escaped, not the A1 range.
Summary
Adds a dedicated transformation system for Google Sheets with separate AI prompts that understand 2D array structures, per-metric transformers (not shared across orgs), and smart chart type selection.
Key Changes
gsheets/module with dedicated prompts for data ingestion and chart generationgsheets-*templates to dedicated generators while other templates remain cached/sharedSummary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.