Skip to content

fix(editor): keep fetches a refresh overtook out of the refreshed column and field path caches - #3094

Merged
datlechin merged 3 commits into
mainfrom
fix/completion-field-paths-stale-scope
Sep 23, 2026
Merged

datlechin merged 3 commits into
mainfrom
fix/completion-field-paths-stale-scope

Conversation

@datlechin

@datlechin datlechin commented Sep 23, 2026

Copy link
Copy Markdown
Member

The defect

Two SQLSchemaProvider caches wrote stale data after a refresh:

  • fieldPaths(for:) samples a MongoDB collection's field paths for MQL completion and the filter panel's column menu. resetForDatabase clears that cache and the in-flight tasks, but a sample already in flight did not notice. When it finished it wrote the pre-refresh paths into the refreshed cache, and set fieldPathTasks[key] = nil even when that slot now held a newer sample started after the refresh. So a third caller could start a duplicate $sample. Shipped since v0.65.0 (feat(plugin-mongodb): nested field paths, write options, and MQL formatting #2105).
  • getColumns(for:schema:) had the same stale write with no in-flight task at all. A column fetch that a refresh overtook wrote the pre-refresh columns into the refreshed column cache, so column completion showed dropped or renamed columns until the next refresh. If the eager preload filled the key during the await, the key was also appended to columnAccessOrder twice, which skews LRU eviction.

The fix

The task-identity check onDemandTables(inSchema:) got in #3070, applied to both:

  • fieldPaths: after the await, a sample only clears its slot and writes the cache while fieldPathTasks[key] is still its own task. Sampling errors are now logged with publicLogShape instead of being swallowed by try?.
  • getColumns: gets an in-flight task per cache key (columnTasks), cleared by resetForDatabase and clearColumnCache, with the same check. Concurrent requests for one table now share a fetch, and updateValue keeps a key from being appended to the access order twice.

An overtaken fetch still answers its own caller. Cancelling overtaken tasks and returning [] to that caller are separate follow-ups.

Tests

Two cases in SQLSchemaProviderUnqualifiedScopeTests, one per cache. Each holds the first fetch, refreshes, starts a second fetch, releases the first, then checks that a third caller joins the second rather than starting another.

  • overtakenFieldPathSampleLeavesTheRefreshedScopeAlone: with its guard line removed it fails and the other 10 cases pass.
  • overtakenColumnFetchLeavesTheRefreshedScopeAlone: with its guard line removed it fails and the other 11 cases pass.

The shared test helper (ScriptedFetch) now holds any number of calls, and the two "database switch" test names say "refresh", which is the case that actually reaches one provider twice.

With both fixes, the provider suites (SQLSchemaProviderUnqualifiedScopeTests, SQLSchemaProviderTests, SQLSchemaProviderFallbackTests, CachedColumnOrderTests, MongoCompletionCaseTests, SQLCompletionProviderTests, SchemaProviderRegistryTests, CopilotPreambleBuilderTests, SchemaContextForAITests): 223 executed, 223 passed. swiftlint lint --strict on both Swift files: 0 violations.

Review

Skill(code-review) read the fieldPaths diff. Fixed from it:

  • The CHANGELOG line and test names described a database switch, but providers are per scope, so the race is a refresh inside one scope.
  • The test did not cover the cleared-newer-task half.
  • A typealias shim in the test file.
  • Errors swallowed without a log line.

It also found the same stale write in getColumns, now fixed here. The getColumns change has not had a separate review.

Found while building #3070.

@datlechin datlechin changed the title fix(editor): keep a field path sample a refresh overtook out of the refreshed cache fix(editor): keep fetches a refresh overtook out of the refreshed column and field path caches Sep 23, 2026
@datlechin
datlechin merged commit e031ba6 into main Sep 23, 2026
8 of 9 checks passed
@datlechin
datlechin deleted the fix/completion-field-paths-stale-scope branch September 23, 2026 19:24
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