refactor: use discriminated union for all multi-operation agent tools#814
Conversation
…args
Replace flat z.object schemas with z.discriminatedUnion('operation', [...])
across all 9 multi-operation agent tools: docx, excel, pdf, text, pptx,
product_read, customer_read, workflow_read, and database_schema.
This provides better type safety via TypeScript narrowing, enforces
required fields per operation at the schema level, and removes redundant
runtime validation checks.
- product_read_tool: add .nonempty() to productIds schema to reject empty arrays (previously caught by runtime check) - excel_tool: remove unnecessary `as const` cast and dead `?? 'unknown.xlsx'` fallback (fileName is now required)
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR refactors argument schemas across nine agent tool files by replacing flat object schemas with discriminated unions based on the operation field. Each operation variant now defines its own required and optional fields rather than using a single schema with optional fields. Runtime validation checks for required fields (e.g., presence of customerId, fileName, workflowId) are removed, delegating validation to the schema structure. Handler logic is simplified to rely on the discriminated union for type narrowing and field availability. Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes The PR applies a consistent schema refactoring pattern across nine heterogeneous files (customer, database, docx, excel, pdf, pptx, text, product, and workflow tools), each with distinct operation variants and field requirements. While the refactoring follows a unified pattern, reviewers must verify that each discriminated union correctly captures per-operation field requirements, that all removed runtime checks are appropriately replaced by schema validation, and that handler branching logic correctly aligns with the new schema structure. The spread across multiple tools and the mixture of schema, validation, and control flow changes increases cognitive load despite the consistency of the pattern. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Summary
z.object()schemas withz.discriminatedUnion('operation', [...])across all 9 multi-operation agent tools, matching the pattern already established inimage_tool.ts.nonempty()validation toproductIdsinproduct_read_toolto preserve fail-fast behavior for empty arraysTools refactored
fileNamerequired in generate;fileId/user_inputrequired in parsefileName/sheetsrequired in generate;fileIdrequired in parsefileName/sourceType/contentrequired in generate;fileId/user_inputrequired in parsefilename/contentrequired in generate;fileId/user_inputrequired in parsefileName/slidesContentrequired in generate;fileId/user_inputrequired in parseproductIds(nonempty) required in get_by_idcustomerIdrequired in get_by_id;emailrequired in get_by_emailtableNamerequired in get_table_schemaTest plan
npx tsc --noEmit— zero type errorsbun run --filter @tale/platform lint— zero warnings/errors🤖 Generated with Claude Code
Summary by CodeRabbit