Skip to content

do not check whether rule application is memoized if it is only interesting to see whether the line is obsolete#136

Merged
georghinkel merged 2 commits intomainfrom
anytext-fixes
Mar 23, 2026
Merged

do not check whether rule application is memoized if it is only interesting to see whether the line is obsolete#136
georghinkel merged 2 commits intomainfrom
anytext-fixes

Conversation

@georghinkel
Copy link
Copy Markdown
Contributor

@georghinkel georghinkel commented Mar 23, 2026

Summary by MergeMonkey

  • Enhancements:
    • Added position-aware IterateLiterals overloads to support efficient position-based literal iteration.
    • Added GetEssentialInnerRuleApplication method to traverse to essential inner rule applications.
  • Squashed Bugs:
    • Optimized obsolete line detection by checking position before expensive memoization lookups.
    • Removed unnecessary column existence checks when determining if lines are obsolete.

@mergemonkeyhq
Copy link
Copy Markdown

mergemonkeyhq bot commented Mar 23, 2026

Risk AssessmentNEEDS-TESTING

Focus areas: ChangeTracker obsolescence logic · Matcher IsObsoleted refactoring · Essential() method traversal · IterateLiterals position-based iteration

Assessment: Performance optimization changing obsolescence check logic and adding new method overloads.

Walkthrough

The PR optimizes obsolete line detection by introducing an Essential() method that traverses to the core rule application, bypassing memoization checks. When checking if a line is obsolete, the code now first validates position-based conditions (whether the rule application falls within edit boundaries) before invoking the more expensive memoization logic. This reduces unnecessary computation when only determining line obsolescence is needed.

Changes

Files Summary
ChangeTracker Optimization
AnyText/AnyText.Core/ChangeTracker.cs
Uses Essential() method to skip memoization checks when determining if lines are obsolete, improving performance of list migration calculations.
Matcher Obsolescence Logic
AnyText/AnyText.Core/Matcher.cs
Refactored IsObsoleted to check position-based conditions first; added overload accepting fromPosition parameter; removed redundant column existence checks.
Rule Application Core Changes
AnyText/AnyText.Core/Rules/RuleApplication.cs, Rule.cs
Added Essential() method to get inner rule application and new IterateLiterals overloads supporting position-based iteration.
Rule Type Implementations
AnyText/AnyText.Core/Rules/ChoiceRule.cs, QuoteRule.cs, SequenceRule.cs, SingleRuleApplication.cs, MultiRuleApplication.cs, LiteralRuleApplication.cs, FailedRuleApplication.cs, FailedChoiceRuleApplication.cs, InheritedFailRuleApplication.cs, RecoveredSequenceRuleApplication.cs
Implemented GetEssentialInnerRuleApplication and new IterateLiterals overloads with ParsePosition parameter across all rule application types.
Test Cleanup
AnyText/Tests/AnyText.Tests/Languages/ListExpressionTests.cs
Minor test assertion cleanup and removal of unnecessary type casts.

Sequence Diagram

sequenceDiagram
    participant C as ChangeTracker
    participant M as Matcher
    participant RA as RuleApplication
    participant R as Rule
    participant L as LiteralRuleApplication
    C->>RA: call Essential()
    RA->>R: GetEssentialInnerRuleApplication()
    R-->>RA: return essential inner
    C->>M: IsObsoleted(essentialApp, edit)
    alt position within edit range
        M-->>C: return true (obsoleted)
    else position after edit
        M->>L: IterateLiterals(fromPosition)
        L-->>M: check literal obsolescence
        M-->>C: return memoization result
    end
Loading

Dig Deeper With Commands

  • /review <file-path> <function-optional>
  • /chat <file-path> "<question>"
  • /roast <file-path>

Runs only when explicitly triggered.

Comment thread AnyText/AnyText.Core/Rules/MultiRuleApplication.cs
Comment thread AnyText/AnyText.Core/Rules/RecoveredSequenceRuleApplication.cs
Comment thread AnyText/AnyText.Core/Rules/LiteralRuleApplication.cs
@mergemonkeyhq
Copy link
Copy Markdown

mergemonkeyhq bot commented Mar 23, 2026

Actionable Comments Posted: 3

🧾 Coverage Summary
✔️ Covered (16 files)
- AnyText/AnyText.Core/ChangeTracker.cs
- AnyText/AnyText.Core/Matcher.cs
- AnyText/AnyText.Core/Rules/ChoiceRule.cs
- AnyText/AnyText.Core/Rules/FailedChoiceRuleApplication.cs
- AnyText/AnyText.Core/Rules/FailedRuleApplication.cs
- AnyText/AnyText.Core/Rules/InheritedFailRuleApplication.cs
- AnyText/AnyText.Core/Rules/LiteralRuleApplication.cs
- AnyText/AnyText.Core/Rules/MultiRuleApplication.cs
- AnyText/AnyText.Core/Rules/QuoteRule.cs
- AnyText/AnyText.Core/Rules/RecoveredSequenceRuleApplication.cs
- AnyText/AnyText.Core/Rules/Rule.cs
- AnyText/AnyText.Core/Rules/RuleApplication.cs
- AnyText/AnyText.Core/Rules/SequenceRule.cs
- AnyText/AnyText.Core/Rules/SingleRuleApplication.cs
- AnyText/AnyText.history
- AnyText/Tests/AnyText.Tests/Languages/ListExpressionTests.cs

@georghinkel georghinkel merged commit 12c453b into main Mar 23, 2026
1 check passed
@georghinkel georghinkel deleted the anytext-fixes branch March 23, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant