feat: Implement new config param, allowCircularReferences #1541
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new configuration parameter allowCircularReferences that enables handling of circular dependencies in formulas with iterative computation instead of throwing CYCLE errors. When enabled, the system performs up to 100 iterations to resolve circular references to stable values.
Key changes:
- Adds
allowCircularReferencesboolean configuration parameter with proper validation - Implements iterative circular dependency resolution algorithm in the Evaluator
- Adds comprehensive test coverage for various circular reference scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/ConfigParams.ts | Adds allowCircularReferences parameter interface definition |
| src/Config.ts | Implements configuration parameter with validation and default value |
| src/Evaluator.ts | Adds circular dependency resolution logic with 100-iteration limit |
| test/circular-dependencies.spec.ts | Comprehensive test suite covering circular reference scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thank you for sharing the pull request and signing our CLA. P.S. You have an impressive GitHub activity! |
There was a problem hiding this comment.
@mountEvarus, thank you for a solid implementation and many good-quality unit tests. It's almost ready to merge, but there are a few details that need to be tweaked.
| * @default false | ||
| * @category Engine | ||
| */ | ||
| allowCircularReferences: boolean, |
There was a problem hiding this comment.
| allowCircularReferences: boolean, | |
| enableIterativeCalculation: boolean, |
I'd rename this option to make it similar to the analogues feature in other spreadsheet software.
| import {Statistics, StatType} from './statistics' | ||
|
|
||
| export class Evaluator { | ||
| private readonly iterationCount = 100 |
There was a problem hiding this comment.
This should be configurable. I'd introduce a new flag iterativeCalculationLimit
| } | ||
| }) | ||
|
|
||
| for (let i = 0; i < cycles; i++) { |
There was a problem hiding this comment.
This loop should stop early if the cycle diverges, similarly to the analogues feature in Google Sheets.
I'd introduce a new flag iterativeCalculationThreshold.
3 participants
Context
Implementing the handling of circular dependencies/references with a flag.
How did you test your changes?
I have created a test file with several scenarios, and matched the results to what should happen in 100 cycles.
Types of changes
Checklist: