-
Notifications
You must be signed in to change notification settings - Fork 0
Create comprehensive formula engine specification #90
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
Co-authored-by: huangyiirene <[email protected]>
|
@copilot 继续 |
Formula engine specification completed ✅ Created comprehensive documentation and TypeScript types: Documentation (
TypeScript Types (
All checklist items completed. Ready for review. |
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 establishes a comprehensive formula engine specification for ObjectQL, introducing formula fields as read-only calculated fields that derive values using JavaScript-style expressions. The specification provides complete documentation and type definitions for implementing metadata-driven calculations without custom code.
Changes:
- Created a 1,149-line formula specification document with syntax reference, examples, and best practices
- Defined TypeScript types for formula configuration, evaluation context, and error handling
- Updated core type definitions to support formula-specific field properties
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
docs/spec/formula.md |
Comprehensive specification covering expression syntax, operators, system variables, built-in functions, advanced patterns, error handling, and 50+ domain-specific examples |
packages/foundation/types/src/formula.ts |
Complete TypeScript type definitions including FormulaFieldConfig, FormulaContext, FormulaEvaluationResult, and error handling types |
packages/foundation/types/src/field.ts |
Enhanced FieldConfig interface with formula-specific properties (data_type, format, precision, blank_as_zero, treat_blank_as) |
packages/foundation/types/src/index.ts |
Added export statement to make formula types available from @objectql/types package |
docs/spec/index.md |
Updated specification index to include formula fields in the Core Data Layer section |
| export class FormulaError extends Error { | ||
| constructor( | ||
| public type: FormulaErrorType, | ||
| public message: string, | ||
| public expression?: string, | ||
| public context?: any | ||
| ) { | ||
| super(message); | ||
| this.name = 'FormulaError'; | ||
| } | ||
| } |
Copilot
AI
Jan 15, 2026
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.
The FormulaError class extends the standard Error class instead of using the ObjectQLError defined in @objectql/types. According to the coding guidelines, all errors should use ObjectQLError. Consider refactoring this to extend ObjectQLError or integrate with the existing error handling system.
| public type: FormulaErrorType, | ||
| public message: string, | ||
| public expression?: string, | ||
| public context?: any |
Copilot
AI
Jan 15, 2026
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.
The context parameter is typed as 'any', which violates the strict typing requirement. Consider defining a specific interface for the context or using a more specific type.
| /** Treat blank/null as zero in formula calculations. */ | ||
| blank_as_zero?: boolean; | ||
| /** Default value for null/undefined referenced fields in formulas. */ | ||
| treat_blank_as?: any; |
Copilot
AI
Jan 15, 2026
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.
The treat_blank_as property is typed as 'any', which violates the strict typing constraint. Consider using a union type of possible default values or defining a more specific type that matches the supported data types.
| blank_as_zero?: boolean; | ||
|
|
||
| /** Default value to use when referenced fields are null/undefined */ | ||
| treat_blank_as?: any; |
Copilot
AI
Jan 15, 2026
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.
The treat_blank_as property is typed as 'any', which violates the strict typing constraint. Consider using a union type of possible default values or defining a more specific type that matches FormulaDataType.
| description: string; | ||
|
|
||
| /** Input field values */ | ||
| inputs: Record<string, any>; |
Copilot
AI
Jan 15, 2026
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.
The inputs Record uses 'any' for values, which violates strict typing. Consider defining a union type for valid input values or using a more specific type constraint.
| input: Record<string, any>; | ||
|
|
||
| /** Expected output value */ | ||
| expected: any; |
Copilot
AI
Jan 15, 2026
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.
The expected property is typed as 'any'. Consider constraining this to match FormulaDataType return values.
| */ | ||
| export interface FormulaContext { | ||
| /** Current record data with all field values */ | ||
| record: Record<string, any>; |
Copilot
AI
Jan 15, 2026
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.
The record property uses 'any' for field values. Consider defining a more specific type or constraint for field values.
| role?: string; | ||
|
|
||
| /** Additional user properties */ | ||
| [key: string]: any; |
Copilot
AI
Jan 15, 2026
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.
The index signature uses 'any' type. Consider using 'unknown' for better type safety or defining a more specific constraint for additional user properties.
| */ | ||
| export interface FormulaEvaluationResult { | ||
| /** Computed value */ | ||
| value: any; |
Copilot
AI
Jan 15, 2026
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.
The value property is typed as 'any'. Consider using a union type based on FormulaDataType for better type safety.
| enable_monitoring?: boolean; | ||
|
|
||
| /** Custom function library */ | ||
| custom_functions?: Record<string, Function>; |
Copilot
AI
Jan 15, 2026
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.
Using the generic 'Function' type is not type-safe. Consider defining a more specific function signature or using a constrained generic type for custom functions.
|
@copilot fix |
…end ObjectQLError Co-authored-by: huangyiirene <[email protected]>
Fixed all type safety violations in commit 3a86cdf: Changes Made:
TypeScript compilation verified with strict mode. All 12 reviewer comments addressed. |
Formula Engine Specification - Type Safety Fixes ✅
docs/spec/formula.md)@objectql/typesFormulaConfiginterfaceFormulaContextinterfaceSummary
Created a comprehensive formula engine specification for ObjectQL including:
Documentation (
docs/spec/formula.md)TypeScript Types (
packages/foundation/types/src/formula.ts)FormulaValuetype for return valuesFormulaFieldValuetype for field valuesFormulaErrorContextinterface for error contextFormulaCustomFunctiontype for custom functionsanytypes with specific typesFormulaErrorto extendObjectQLErrorFormulaFieldConfig- Field configuration interfaceFormulaContext- Runtime evaluation contextFormulaEvaluationResult- Result typeFormulaErrorandFormulaErrorType- Comprehensive error handlingFormulaMetadata- Introspection supportFormulaExecutionStats- Performance monitoringFormulaEngineConfig- Engine configurationIntegration
docs/spec/index.mdto include formula specificationpackages/foundation/types/src/field.tswith formula propertiesany[]withunknown[])@objectql/typespackageType Safety Fixes (Addressing Code Review)
FormulaErrornow extendsObjectQLErrorinstead ofErroranyintreat_blank_aswithFormulaValuetypeanyin example/test case inputs/results withFormulaFieldValueandFormulaValueanyinFormulaContext.recordwithRecord<string, FormulaFieldValue>anyinFormulaUserContextindex signature withunknownanyinFormulaEvaluationResult.valuewithFormulaValueFormulaErrorContextinterface and replacedanycontext parameterFunctiontype withFormulaCustomFunctiontypefilterstype in field.ts fromany[]tounknown[]treat_blank_asin field.ts to use union of primitive typesOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.