Skip to content

Conversation

@richardm90
Copy link
Contributor

@richardm90 richardm90 commented Oct 31, 2025

The Problem

I've experienced odd behaviour when changing RPG source in that the linter would suddenly display a "Variable name casing does not match definition" warning at various points within the source I was editing. Although this appears to be a linter issue it goes further, the tokens appear to lose their offset within the source so when you use something like F2 to rename a symbol it loses track of where those symbols are and renames the wrong text in the source.

  • I notice the problem most often when adding additional /include files - I do use a lot of /include files.
  • I've not noticed it whilst editing local source only remote source.

I was able to pull together a very simple example that replicates the problem, which I've also created a video that demonstrates the problem as it's not easy to understand from the description.

rm82test-2025-10-30_20.45.00.mp4

Full disclosure I leaned on Claude Code to help fix the problem and produce the documentation of the changes.

The extension was experiencing race condition offset issues that manifested as false linter errors during live editing:

  • Immediate parsing on every keystroke - The parser was invoked synchronously on every document change
  • Multiple concurrent parse operations - When users typed quickly, multiple parse operations would run simultaneously on different document versions
  • Stale/incorrect offsets - This resulted in duplicate definitions with incorrect line/column offsets in the parser cache
  • False linter diagnostics - The linter would report incorrect errors (e.g., variable name casing issues) based on stale offset data
  • Poor user experience - Diagnostics would flicker, show false warnings, and create confusion during normal editing

The Solution

The fix implements a comprehensive debouncing and synchronization mechanism with multiple layers of protection:

  1. Debouncing (server.ts:326-386)

    • Added a 300ms debounce timer to delay parsing until the user pauses typing
    • Each new keystroke clears the previous timer and restarts the countdown
    • This prevents triggering expensive parse operations on every character typed
  2. Parse State Tracking (lines 327-333)

    const documentParseState: {
      [uri: string]: {
        timer?: NodeJS.Timeout,    // The debounce timer
        parseId: number,            // Incrementing counter for each change
        parsing: boolean            // Flag to prevent concurrent parses
      }
    } = {};
    
  3. Parse ID Validation (lines 351-353, 378)

    • Each document change increments a parseId counter
    • Parse operations capture the current parseId before starting
    • Diagnostics are only updated if the parseId still matches when the parse completes
    • This invalidates any slow/stale parse operations that finish after newer changes have been made
  4. Concurrent Parse Prevention (lines 359-362)

    • A parsing boolean flag prevents multiple simultaneous parse operations for the same document
    • If a parse is already in progress when the timer fires, the new parse is skipped
    • This prevents race conditions even if the debounce timer fires multiple times
  5. Error Handling (lines 381-384)

    • Added .catch() block to gracefully handle parse errors
    • Ensures the parsing flag is properly reset even if parsing fails
    • Logs errors to console for debugging
  6. Minor Cleanup

    • Removed trailing whitespace on lines 206, 219, and 297

Technical Flow

  1. User types → Document change event fires
  2. Clear old timer → Cancel any pending parse operation
  3. Increment parseId → Invalidate any in-flight parses
  4. Start new timer (300ms) → Wait for typing to pause
  5. Timer fires → Check if already parsing, skip if so
  6. Set parsing flag → Prevent concurrent parses
  7. Parse document → Run async parse operation
  8. Validate parseId → Only update diagnostics if this is still the latest version
  9. Reset parsing flag → Allow next parse

Key Improvements Over Initial Version

The uncommitted changes add several critical improvements beyond the basic debouncing:

  1. Parse ID validation prevents stale results from updating diagnostics
  2. Concurrent parse prevention stops race conditions entirely
  3. Better error handling ensures state consistency
  4. Faster response (300ms vs 500ms) improves perceived performance while maintaining stability

TODO

  • I am still testing the changes
  • I need to look at unit tests I can create though I'm pretty sure that won't be possible for the race-condition
  • I have noticed a couple of other things along the way that I want to look at:
    • The /include files are loaded a lot, the extension seems to reload the same file many, many times. I connect over VPN for a lot of my clients and loading source can take a while.
    • File declarations don't always load the results in the outline view
    • There doesn't appear to be a way to reload the content of the /include files, if they've changed
    • There is no feedback on whether the current source is still being parsed
    • There is an option in the Outline view to reload cache though this only appears to reload file declarations, maybe this should reload all /includes too
    • How often is the outline view updated? It doesn't always appear to be up to date with recent source changes

Checklist

  • have tested my change
  • updated relevant documentation
  • Remove any/all console.logs I added
  • eslint is not complaining
  • have added myself to the contributors' list in the README
  • for feature PRs: PR only includes one feature enhancement.

richardm90 and others added 2 commits October 30, 2025 15:51
Prevents multiple concurrent parse operations during live editing that
were causing duplicate definitions with incorrect offsets, resulting in
false linter errors about variable name casing.
Improves the offset issue fix by adding parse ID tracking to invalidate
stale parse operations, a parsing flag to prevent concurrent parses,
error handling, and reduces debounce time from 500ms to 300ms for better
responsiveness.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@worksofliam worksofliam self-requested a review November 3, 2025 03:45
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.

1 participant