Skip to content

fix: auto-invalidate location cache after document mutations - #420

Draft
toddr-bot wants to merge 2 commits into
masterfrom
koan.toddr.bot/fix-issue-91
Draft

fix: auto-invalidate location cache after document mutations#420
toddr-bot wants to merge 2 commits into
masterfrom
koan.toddr.bot/fix-issue-91

Conversation

@toddr-bot

@toddr-bot toddr-bot commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

When a PPI document is modified via insert_before, insert_after, remove, replace, add_element, or prune, cached token locations become stale but are not automatically invalidated. Users had to manually call flush_locations() after every mutation — error-prone and undocumented on the mutation methods themselves.

This PR adds a _locations_dirty flag that is set on the owning PPI::Document whenever a mutation occurs. When location() is next called, it detects the dirty flag and transparently flushes + re-indexes all positions. The internal __add_element (used only during initial parsing) is unchanged, avoiding any performance impact on document construction.

Fixes #91

Changes

  • PPI::Node: Added __notify_locations_dirty helper; called from add_element, remove_child, __insert_before_child, __insert_after_child, and __replace_child
  • PPI::Document: index_locations flushes stale caches when _locations_dirty flag is set
  • PPI::Element: _ensure_location_present checks the dirty flag before returning cached locations
  • t/ppi_location_dirty.t: New test file covering all 6 mutation paths
  • t/ppi_element_flush.t: Updated existing assertions that previously expected stale locations

Test plan

  • New test t/ppi_location_dirty.t verifies auto-invalidation for: insert_before, insert_after, remove, replace, add_element, prune
  • Updated t/ppi_element_flush.t to expect fresh (auto-refreshed) locations instead of stale ones
  • Full test suite passes (70 files, 53097 tests)

Generated by Kōan /fix


Quality Report

Changes: 5 files changed, 153 insertions(+), 28 deletions(-)

Code scan: clean

Tests: skipped

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

toddr-bot and others added 2 commits April 27, 2026 05:39
…ions

Locations cached by index_locations() become stale after mutations
(insert_before, insert_after, remove, replace, add_element, prune)
but are not automatically invalidated. Users must manually call
flush_locations(), which is error-prone and undocumented on mutation
methods.

These tests verify that location() returns correct values after each
type of mutation. All mutation tests are marked $TODO and fail,
demonstrating the bug described in GitHub issue #91.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a PPI document is modified via insert_before, insert_after, remove,
replace, add_element, or prune, cached token locations become stale.
Previously users had to manually call flush_locations() after every
mutation, which was error-prone and undocumented on the mutation methods.

The fix adds a _locations_dirty flag to PPI::Document that is set by a
__notify_locations_dirty helper called from all Node mutation methods
(add_element, remove_child, __insert_before_child, __insert_after_child,
__replace_child). When location() is subsequently called via
_ensure_location_present, it detects the dirty flag and triggers
flush_locations + index_locations to recalculate all positions.

The internal __add_element (used only during initial parsing) is not
modified, avoiding any performance impact on document construction.

Fixes #91

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

location cache index needs to be invalidated after actions that change a document

1 participant