From 8fa46173571495134b1968035154b19127b802e7 Mon Sep 17 00:00:00 2001 From: Vamsi-klu Date: Tue, 10 Nov 2026 04:14:44 +0000 Subject: [PATCH 1/2] fix(vscode): follow active model paths on Windows Signed-off-by: Vamsi-klu --- vscode/bus/src/callbacks.ts | 2 + vscode/extension/CHANGELOG.md | 3 +- .../src/webviews/lineagePanel.test.ts | 128 +++++++++ vscode/extension/src/webviews/lineagePanel.ts | 7 +- vscode/extension/tests/lineage.spec.ts | 43 ++- vscode/react/src/hooks/eventBus.tsx | 1 + vscode/react/src/pages/lineage.spec.tsx | 261 ++++++++++++++++++ vscode/react/src/pages/lineage.tsx | 91 +++--- vscode/react/src/utils/path.spec.ts | 69 +++++ vscode/react/src/utils/path.ts | 19 ++ 10 files changed, 576 insertions(+), 48 deletions(-) create mode 100644 vscode/extension/src/webviews/lineagePanel.test.ts create mode 100644 vscode/react/src/pages/lineage.spec.tsx create mode 100644 vscode/react/src/utils/path.spec.ts create mode 100644 vscode/react/src/utils/path.ts diff --git a/vscode/bus/src/callbacks.ts b/vscode/bus/src/callbacks.ts index 82d8231add..f6165ed66d 100644 --- a/vscode/bus/src/callbacks.ts +++ b/vscode/bus/src/callbacks.ts @@ -29,6 +29,7 @@ export type VSCodeCallbackShape = Record export type VSCodeCallback = { changeFocusOnFile: { path: string + isWindows?: boolean } savedFile: { fileUri: string @@ -47,6 +48,7 @@ export type RPCMethods = { params: {} result: { fileUri?: string + isWindows?: boolean } } api_query: { diff --git a/vscode/extension/CHANGELOG.md b/vscode/extension/CHANGELOG.md index fa2519edaf..04cf8f1e24 100644 --- a/vscode/extension/CHANGELOG.md +++ b/vscode/extension/CHANGELOG.md @@ -6,4 +6,5 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] -- Initial release \ No newline at end of file +- Fixed lineage selection when Windows paths differ in casing or separators, using the extension host's platform for remote workspaces. +- Initial release diff --git a/vscode/extension/src/webviews/lineagePanel.test.ts b/vscode/extension/src/webviews/lineagePanel.test.ts new file mode 100644 index 0000000000..0d6fad9504 --- /dev/null +++ b/vscode/extension/src/webviews/lineagePanel.test.ts @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TextEditor, Uri, WebviewView } from 'vscode' +import type { LSPClient } from '../lsp/lsp' +import { LineagePanel } from './lineagePanel' + +const state = vi.hoisted(() => ({ + isWindows: true, + editor: undefined as TextEditor | undefined, + focus: undefined as ((editor: TextEditor | undefined) => void) | undefined, + saved: undefined as + | ((document: { uri: { toString: () => string } }) => void) + | undefined, + receive: undefined as ((message: unknown) => Promise) | undefined, + post: vi.fn(), + dispose: vi.fn(), +})) + +vi.mock('vscode', () => ({ + Uri: { joinPath: () => 'asset' }, + window: { + get activeTextEditor() { + return state.editor + }, + onDidChangeActiveTextEditor: (callback: typeof state.focus) => { + state.focus = callback + return { dispose: state.dispose } + }, + }, + workspace: { + onDidSaveTextDocument: (callback: typeof state.saved) => { + state.saved = callback + return { dispose: state.dispose } + }, + }, +})) +vi.mock('@bus/result', () => ({ isErr: () => false })) +vi.mock('../utilities/isWindows', () => ({ + get IS_WINDOWS() { + return state.isWindows + }, +})) + +function editor(fsPath: string): TextEditor { + return { document: { uri: { fsPath } } } as TextEditor +} + +function panel() { + const instance = new LineagePanel({} as Uri, {} as LSPClient) + instance.resolveWebviewView({ + webview: { + postMessage: state.post, + asWebviewUri: (uri: unknown) => uri, + onDidReceiveMessage: (callback: typeof state.receive) => { + state.receive = callback + return { dispose: state.dispose } + }, + }, + } as unknown as WebviewView) + return instance +} + +describe('lineage editor path messages', () => { + beforeEach(() => { + vi.clearAllMocks() + state.editor = undefined + }) + + it.each([true, false])( + 'sends extension-host platform %s with active-file RPC and focus events', + async isWindows => { + state.isWindows = isWindows + state.editor = editor('C:\\Project\\A %20#.sql') + const instance = panel() + await state.receive?.({ + key: 'rpcRequest', + payload: { method: 'get_active_file', requestId: 'active', params: {} }, + }) + expect(state.post).toHaveBeenLastCalledWith({ + key: 'rpcResponse', + payload: { + requestId: 'active', + result: { fileUri: 'C:\\Project\\A %20#.sql', isWindows }, + }, + }) + state.focus?.(editor('C:\\Project\\B.sql')) + expect(state.post).toHaveBeenLastCalledWith({ + key: 'vscode_send', + payload: { + key: 'changeFocusOnFile', + payload: { path: 'C:\\Project\\B.sql', isWindows }, + }, + }) + instance.dispose() + expect(state.dispose).toHaveBeenCalledTimes(3) + }, + ) + + it('reports no editor and preserves saved-file URI payloads', async () => { + state.isWindows = true + const instance = panel() + await state.receive?.({ + key: 'rpcRequest', + payload: { method: 'get_active_file', requestId: 'none', params: {} }, + }) + expect(state.post).toHaveBeenLastCalledWith({ + key: 'rpcResponse', + payload: { + requestId: 'none', + result: { fileUri: undefined, isWindows: true }, + }, + }) + state.focus?.(undefined) + expect(state.post).toHaveBeenCalledOnce() + state.saved?.({ + uri: { toString: () => 'file:///c%3A/Project/A%20%23.sql' }, + }) + expect(state.post).toHaveBeenLastCalledWith({ + key: 'vscode_send', + payload: { + key: 'savedFile', + payload: { fileUri: 'file:///c%3A/Project/A%20%23.sql' }, + }, + }) + instance.dispose() + }) +}) diff --git a/vscode/extension/src/webviews/lineagePanel.ts b/vscode/extension/src/webviews/lineagePanel.ts index a3c307d204..d396f264e6 100644 --- a/vscode/extension/src/webviews/lineagePanel.ts +++ b/vscode/extension/src/webviews/lineagePanel.ts @@ -11,6 +11,7 @@ import { import { getWorkspaceFolders } from '../utilities/common/vscodeapi' import { LSPClient } from '../lsp/lsp' import { isErr } from '@bus/result' +import { IS_WINDOWS } from '../utilities/isWindows' export class LineagePanel implements WebviewViewProvider, Disposable { public static readonly viewType = 'sqlmesh.lineage' @@ -47,7 +48,10 @@ export class LineagePanel implements WebviewViewProvider, Disposable { key: 'vscode_send', payload: { key: 'changeFocusOnFile', - payload: { path: editor.document.uri.fsPath }, + payload: { + path: editor.document.uri.fsPath, + isWindows: IS_WINDOWS, + }, }, }) } @@ -144,6 +148,7 @@ export class LineagePanel implements WebviewViewProvider, Disposable { requestId, result: { fileUri: active_file, + isWindows: IS_WINDOWS, }, }, } diff --git a/vscode/extension/tests/lineage.spec.ts b/vscode/extension/tests/lineage.spec.ts index 66e3048246..3db8a2325f 100644 --- a/vscode/extension/tests/lineage.spec.ts +++ b/vscode/extension/tests/lineage.spec.ts @@ -1,9 +1,10 @@ -import { test, Page } from './fixtures' +import { test, expect, Page } from './fixtures' import path from 'path' import fs from 'fs-extra' import os from 'os' import { openLineageView, + openFile, openServerPage, SUSHI_SOURCE_PATH, waitForLoadedSQLMesh, @@ -19,7 +20,7 @@ async function testLineageWithProjectPath(page: Page): Promise { await waitForLoadedSQLMesh(page) } -test('Lineage panel renders correctly - no project path config (default)', async ({ +test('Lineage panel follows the initial editor and model switches - no project path config (default)', async ({ page, sharedCodeServer, tempDir, @@ -28,7 +29,28 @@ test('Lineage panel renders correctly - no project path config (default)', async await createPythonInterpreterSettingsSpecifier(tempDir) await openServerPage(page, tempDir, sharedCodeServer) - await testLineageWithProjectPath(page) + await openFile(page, 'models/waiters.py') + await waitForLoadedSQLMesh(page) + await openLineageView(page) + + // The main model has the brand ring; checking it distinguishes selection + // from a model that merely appears as a dependency in the graph. + const expectMainModel = async (name: string) => { + await expect + .poll(async () => { + for (const frame of page.frames()) { + const main = frame.locator('.react-flow__node .ring-brand-50') + if (await main.locator(`[title*="${name}"]`).count()) return true + } + return false + }) + .toBe(true) + } + await expectMainModel('waiters') + await openFile(page, 'models/orders.py') + await expectMainModel('orders') + await openFile(page, 'models/waiters.py') + await expectMainModel('waiters') }) test('Lineage panel renders correctly - relative project path', async ({ @@ -129,7 +151,6 @@ test('Lineage panel renders correctly - absolute path project outside of workspa await testLineageWithProjectPath(page) }) -// These work on local machine when debuggin but not on CI, so skipping for now test('Lineage panel renders correctly - multiworkspace setup', async ({ page, sharedCodeServer, @@ -174,7 +195,12 @@ test('Lineage panel renders correctly - multiworkspace setup', async ({ ) await openServerPage(page, workspaceFilePath, sharedCodeServer) - await page.reload() + await expect( + page.getByRole('treeitem', { name: 'sushi1', exact: true }), + ).toBeVisible() + await expect( + page.getByRole('treeitem', { name: 'sushi2', exact: true }), + ).toBeVisible() await testLineageWithProjectPath(page) }) @@ -228,6 +254,11 @@ test('Lineage panel renders correctly - multiworkspace setup reversed', async ({ ) await openServerPage(page, workspaceFilePath, sharedCodeServer) - await page.reload() + await expect( + page.getByRole('treeitem', { name: 'sushi1', exact: true }), + ).toBeVisible() + await expect( + page.getByRole('treeitem', { name: 'sushi2', exact: true }), + ).toBeVisible() await testLineageWithProjectPath(page) }) diff --git a/vscode/react/src/hooks/eventBus.tsx b/vscode/react/src/hooks/eventBus.tsx index 308886b6fc..1c23c554c3 100644 --- a/vscode/react/src/hooks/eventBus.tsx +++ b/vscode/react/src/hooks/eventBus.tsx @@ -14,6 +14,7 @@ import { export type EventMap = { changeFocusedFile: { fileUri: string + isWindows?: boolean } savedFile: { fileUri: string diff --git a/vscode/react/src/pages/lineage.spec.tsx b/vscode/react/src/pages/lineage.spec.tsx new file mode 100644 index 0000000000..3105f98350 --- /dev/null +++ b/vscode/react/src/pages/lineage.spec.tsx @@ -0,0 +1,261 @@ +// SPDX-License-Identifier: Apache-2.0 + +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { PropsWithChildren } from 'react' +import { EventBusProvider } from '@/hooks/eventBus' +import { QueryClient } from '@tanstack/react-query' +import { LineagePage } from './lineage' + +const { api, rpc, send, graph } = vi.hoisted(() => ({ + api: vi.fn(), + rpc: vi.fn(), + send: vi.fn(), + graph: {} as { open?: (id: string) => void }, +})) + +vi.mock('@/api', () => ({ useApiModels: api })) +vi.mock('@/utils/rpc', () => ({ useRpc: () => rpc })) +vi.mock('@/hooks/vscode', () => ({ useVSCode: () => send })) +vi.mock('@/components/graph/context', () => ({ + default: ({ + children, + handleClickModel, + }: PropsWithChildren<{ handleClickModel: (id: string) => void }>) => { + graph.open = handleClickModel + return children + }, +})) +vi.mock('@/components/graph/ModelLineage', () => ({ + ModelLineage: ({ model }: { model: { displayName: string } }) => ( +
{model.displayName}
+ ), +})) + +const models = [ + { name: 'first', fqn: 'first', full_path: 'C:\\project\\first.sql' }, + { name: 'second', fqn: 'second', full_path: 'C:\\project\\second.sql' }, +] + +function page() { + return ( + + + + ) +} + +function focus(path: string, isWindows?: boolean) { + act(() => { + window.dispatchEvent( + new MessageEvent('message', { + data: { + key: 'vscode_send', + payload: { key: 'changeFocusOnFile', payload: { path, isWindows } }, + }, + }), + ) + }) +} + +async function selected(name: string) { + await waitFor(() => { + expect(screen.getByTestId('selected-model').textContent).toBe(name) + }) +} + +describe('lineage active-editor selection', () => { + beforeEach(() => { + api.mockReturnValue({ data: models, isLoading: false }) + rpc.mockResolvedValue({ fileUri: models[0].full_path, isWindows: true }) + }) + + afterEach(() => { + cleanup() + vi.clearAllMocks() + vi.restoreAllMocks() + }) + + it('selects the initial Windows editor despite drive case and separators', async () => { + rpc.mockResolvedValue({ fileUri: 'c:/project/second.sql', isWindows: true }) + render(page()) + await selected('second') + }) + + it('follows a Windows editor switch despite directory and filename case', async () => { + render(page()) + await selected('first') + focus('c:/PROJECT/SECOND.SQL', true) + await selected('second') + }) + + it.each([false, undefined])( + 'uses the host flag %s rather than browser platform', + async isWindows => { + vi.spyOn(window.navigator, 'platform', 'get').mockReturnValue('Win32') + rpc.mockResolvedValue({ fileUri: models[0].full_path, isWindows }) + render(page()) + await selected('first') + focus('c:/PROJECT/SECOND.SQL', isWindows) + await selected('first') + focus(models[1].full_path, isWindows) + await selected('second') + }, + ) + + it('uses Windows host matching even with a Linux browser', async () => { + vi.spyOn(window.navigator, 'platform', 'get').mockReturnValue( + 'Linux x86_64', + ) + rpc.mockResolvedValue({ fileUri: 'c:/project/SECOND.SQL', isWindows: true }) + render(page()) + await selected('second') + }) + + it('keeps the selection for ambiguous fallback paths and prefers exact matches', async () => { + const candidates = [ + models[0], + { name: 'upper', fqn: 'upper', full_path: 'C:/A.sql' }, + { name: 'lower', fqn: 'lower', full_path: 'c:/a.sql' }, + ] + api.mockReturnValue({ data: candidates, isLoading: false }) + render(page()) + await selected('first') + focus('C:/a.SQL', true) + await selected('first') + focus('c:/a.sql', true) + await selected('lower') + }) + + it('does not select an arbitrary model for an ambiguous initial path', async () => { + api.mockReturnValue({ + data: [ + { name: 'upper', full_path: 'C:/A.sql' }, + { name: 'lower', full_path: 'c:/a.sql' }, + ], + isLoading: false, + }) + rpc.mockResolvedValue({ fileUri: 'C:/a.SQL', isWindows: true }) + render(page()) + await act(async () => {}) + expect(screen.queryByTestId('selected-model')).toBeNull() + }) + + it.each([undefined, 'C:/project/README.md'])( + 'falls back to the first model at initialization for %s', + async fileUri => { + rpc.mockResolvedValue({ fileUri, isWindows: true }) + render(page()) + await selected('first') + }, + ) + + it('keeps selection when focusing a non-model file', async () => { + render(page()) + await selected('first') + focus('C:/project/README.md', true) + await selected('first') + }) + + it('handles an empty model list followed by delayed model loading', async () => { + api.mockReturnValue({ data: [], isLoading: false }) + const view = render(page()) + expect(rpc).not.toHaveBeenCalled() + expect(screen.queryByTestId('selected-model')).toBeNull() + api.mockReturnValue({ data: models, isLoading: false }) + view.rerender(page()) + await selected('first') + }) + + it('does not let a late initial RPC overwrite the latest A-B-A focus', async () => { + let resolve!: (value: { fileUri: string; isWindows: boolean }) => void + rpc.mockReturnValue( + new Promise(done => { + resolve = done + }), + ) + render(page()) + focus('c:/project/SECOND.SQL', true) + focus('c:/project/FIRST.SQL', true) + await selected('first') + await act(async () => { + resolve({ fileUri: models[1].full_path, isWindows: true }) + }) + await selected('first') + }) + + it('ignores an old initial RPC after a newer model response', async () => { + let resolve!: (value: { fileUri: string; isWindows: boolean }) => void + rpc.mockReturnValueOnce( + new Promise(done => { + resolve = done + }), + ) + rpc.mockResolvedValue({ fileUri: models[1].full_path, isWindows: true }) + const view = render(page()) + api.mockReturnValue({ data: [...models], isLoading: false }) + view.rerender(page()) + await selected('second') + await act(async () => { + resolve({ fileUri: models[0].full_path, isWindows: true }) + }) + await selected('second') + }) + + it('does not revive an initial match after focus moves to a non-model file', async () => { + let resolve!: (value: { fileUri: string; isWindows: boolean }) => void + rpc.mockReturnValue( + new Promise(done => { + resolve = done + }), + ) + render(page()) + focus('C:/project/README.md', true) + await selected('first') + await act(async () => { + resolve({ fileUri: models[1].full_path, isWindows: true }) + }) + await selected('first') + }) + + it('disposes an initial lookup on unmount', async () => { + let resolve!: (value: { fileUri: string }) => void + rpc.mockReturnValue( + new Promise(done => { + resolve = done + }), + ) + const view = render(page()) + view.unmount() + await act(async () => { + resolve({ fileUri: models[0].full_path }) + }) + expect(screen.queryByTestId('selected-model')).toBeNull() + }) + + it('preserves original paths for graph navigation and invalidates on save', async () => { + const invalidate = vi.spyOn(QueryClient.prototype, 'invalidateQueries') + render(page()) + await selected('first') + focus('c:/PROJECT/SECOND.SQL', true) + await selected('second') + graph.open?.(encodeURIComponent(models[1].fqn)) + expect(send).toHaveBeenCalledWith('openFile', { + filePath: models[1].full_path, + }) + act(() => { + window.dispatchEvent( + new MessageEvent('message', { + data: { + key: 'vscode_send', + payload: { + key: 'savedFile', + payload: { fileUri: 'file:///c%3A/project/second.sql' }, + }, + }, + }), + ) + }) + expect(invalidate).toHaveBeenCalledOnce() + }) +}) diff --git a/vscode/react/src/pages/lineage.tsx b/vscode/react/src/pages/lineage.tsx index cca63e5f9a..6047ed2c4c 100644 --- a/vscode/react/src/pages/lineage.tsx +++ b/vscode/react/src/pages/lineage.tsx @@ -9,12 +9,13 @@ import { useApiModels } from '@/api' import LineageFlowProvider from '@/components/graph/context' import { ModelLineage } from '@/components/graph/ModelLineage' import { useVSCode } from '@/hooks/vscode' -import React, { useState } from 'react' +import React, { useRef, useState } from 'react' import { ModelSQLMeshModel } from '@/domain/sqlmesh-model' import { useEventBus } from '@/hooks/eventBus' -import type { VSCodeEvent } from '@bus/callbacks' +import type { RPCMethods, VSCodeEvent } from '@bus/callbacks' import type { Model } from '@/api/client' import { useRpc } from '@/utils/rpc' +import { findModelsByPath } from '@/utils/path' import { type ModelPath, type ModelFullPath, @@ -33,7 +34,10 @@ export function LineagePage() { const payload: VSCodeEvent = event.data.payload switch (payload.key) { case 'changeFocusOnFile': - emit('changeFocusedFile', { fileUri: payload.payload.path }) + emit('changeFocusedFile', { + fileUri: payload.payload.path, + isWindows: payload.payload.isWindows, + }) break case 'savedFile': emit('savedFile', { fileUri: payload.payload.fileUri }) @@ -84,41 +88,37 @@ function Lineage() { error: modelsError, } = useApiModels() const rpc = useRpc() + const focusVersion = useRef(0) React.useEffect(() => { - const fetchFirstTimeModelIfNotSet = async ( - models: Model[], - ): Promise => { - if (!Array.isArray(models)) { - return undefined - } - const activeFile = await rpc('get_active_file', {}) - // @ts-ignore - if (!activeFile.fileUri) { - return models[0].name - } - // @ts-ignore - const filePath: string = activeFile.fileUri - const model = models.find((m: Model) => { - if (!m.full_path) { - return false - } - return m.full_path === filePath - }) - if (model) { - return model.name - } - return undefined + if ( + selectedModel !== undefined || + !Array.isArray(models) || + !models.length + ) { + return } - if (selectedModel === undefined && Array.isArray(models)) { - fetchFirstTimeModelIfNotSet(models).then(modelName => { - if (modelName && selectedModel === undefined) { - setSelectedModel(modelName) - } else { - setSelectedModel(models[0].name) - } - }) + let current = true + const version = focusVersion.current + // get_active_file uses a plain payload on the existing RPC wire protocol. + rpc('get_active_file', {}).then(response => { + if (!current || focusVersion.current !== version) return + const activeFile = + response as unknown as RPCMethods['get_active_file']['result'] + const matches = findModelsByPath( + models, + activeFile.fileUri, + activeFile.isWindows, + ) + setSelectedModel( + previous => + previous ?? + (matches.length > 1 ? undefined : (matches[0] ?? models[0]).name), + ) + }) + return () => { + current = false } - }, [models, selectedModel]) + }, [models, selectedModel, rpc]) const modelsRecord = Array.isArray(models) && @@ -131,12 +131,23 @@ function Lineage() { ) React.useEffect(() => { - const handleChangeFocusedFile = (fileUri: { fileUri: string }) => { - const model = Object.values(modelsRecord).find( - m => m.full_path === fileUri.fileUri, + const handleChangeFocusedFile = (file: { + fileUri: string + isWindows?: boolean + }) => { + focusVersion.current += 1 + const matches = findModelsByPath( + Object.values(modelsRecord), + file.fileUri, + file.isWindows, ) - if (model) { - setSelectedModel(model.name) + if (matches.length < 2) { + setSelectedModel( + previous => + matches[0]?.name ?? + previous ?? + (Array.isArray(models) ? models[0]?.name : undefined), + ) } } diff --git a/vscode/react/src/utils/path.spec.ts b/vscode/react/src/utils/path.spec.ts new file mode 100644 index 0000000000..9940b06b9a --- /dev/null +++ b/vscode/react/src/utils/path.spec.ts @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from 'vitest' +import { findModelsByPath } from './path' + +describe('findModelsByPath', () => { + it.each([ + ['C:\\project\\Model.sql', 'c:/PROJECT/model.sql'], + ['C:/project\\Model.sql', 'c:\\project/MODEL.SQL'], + ['\\\\server\\share\\Model.sql', '//SERVER/share/model.sql'], + ['C:\\space ü\\a%20#b.sql', 'c:/SPACE Ü/a%20#b.sql'], + ['\\\\?\\C:\\Model.sql', '//?/c:/model.sql'], + ])('matches Windows path %s against %s', (full_path, filePath) => { + const model = { full_path } + expect(findModelsByPath([model], filePath, true)).toEqual([model]) + }) + + it.each([ + ['C:/model.sql', 'D:/model.sql'], + ['//server/share/model.sql', '//server/other/model.sql'], + ['//server/share/model.sql', '//other/share/model.sql'], + ['C:/a%20b.sql', 'c:/a b.sql'], + ['C:/a#b.sql', 'c:/a.sql'], + ['\\\\?\\C:\\model.sql', 'C:/model.sql'], + ])('keeps distinct Windows paths %s and %s', (full_path, filePath) => { + expect(findModelsByPath([{ full_path }], filePath, true)).toEqual([]) + }) + + it.each([false, undefined])( + 'preserves POSIX paths with host flag %s', + isWindows => { + expect( + findModelsByPath( + [{ full_path: '/model.sql' }], + '/MODEL.sql', + isWindows, + ), + ).toEqual([]) + expect( + findModelsByPath([{ full_path: '/a\\b.sql' }], '/a/b.sql', isWindows), + ).toEqual([]) + const model = { full_path: '/a\\b.sql' } + expect(findModelsByPath([model], '/a\\b.sql', isWindows)).toEqual([model]) + }, + ) + + it('prefers an exact match over a case-folded candidate', () => { + const models = [{ full_path: 'C:/A.sql' }, { full_path: 'c:/a.sql' }] + expect(findModelsByPath(models, 'c:/a.sql', true)).toEqual([models[1]]) + }) + + it('returns every ambiguous fallback candidate for the caller to retain selection', () => { + const models = [{ full_path: 'C:/A.sql' }, { full_path: 'c:/a.sql' }] + expect(findModelsByPath(models, 'C:/a.SQL', true)).toEqual(models) + }) + + it('skips missing and empty paths', () => { + const models = [ + {}, + { full_path: undefined }, + { full_path: null }, + { full_path: '' }, + ] + expect(findModelsByPath(models, '', true)).toEqual([]) + expect(findModelsByPath(models, undefined, true)).toEqual([]) + expect(findModelsByPath(models, 'C:/a.sql', true)).toEqual([]) + expect(findModelsByPath([], 'C:/a.sql', true)).toEqual([]) + }) +}) diff --git a/vscode/react/src/utils/path.ts b/vscode/react/src/utils/path.ts new file mode 100644 index 0000000000..a9e1da9d7d --- /dev/null +++ b/vscode/react/src/utils/path.ts @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: Apache-2.0 + +/** Exact paths take precedence; multiple Windows fallback matches are ambiguous. */ +export function findModelsByPath( + models: T[], + filePath: string | undefined, + isWindows = false, +): T[] { + if (!filePath) return [] + const exact = models.find(model => model.full_path === filePath) + if (exact) return [exact] + if (!isWindows) return [] + + const normalize = (path: string) => path.replace(/\\/g, '/').toLowerCase() + const normalizedPath = normalize(filePath) + return models.filter( + model => model.full_path && normalize(model.full_path) === normalizedPath, + ) +} From e53201a35b5c61ab3df25c6606efe6543c9a6d11 Mon Sep 17 00:00:00 2001 From: deepinsight coder <32898216+Vamsi-klu@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:41:06 -0800 Subject: [PATCH 2/2] fix(vscode): recover lineage selection after failed lookups Show an explicit model picker after ambiguous or failed active-file lookup, without arbitrarily choosing a model. Cover manual selection and stale RPCs. Signed-off-by: Codex --- vscode/react/src/pages/lineage.spec.tsx | 63 ++++++++++++++++++- vscode/react/src/pages/lineage.tsx | 80 ++++++++++++++++++------- 2 files changed, 121 insertions(+), 22 deletions(-) diff --git a/vscode/react/src/pages/lineage.spec.tsx b/vscode/react/src/pages/lineage.spec.tsx index 3105f98350..b62b2cf290 100644 --- a/vscode/react/src/pages/lineage.spec.tsx +++ b/vscode/react/src/pages/lineage.spec.tsx @@ -1,6 +1,13 @@ // SPDX-License-Identifier: Apache-2.0 -import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { + act, + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { PropsWithChildren } from 'react' import { EventBusProvider } from '@/hooks/eventBus' @@ -139,6 +146,59 @@ describe('lineage active-editor selection', () => { render(page()) await act(async () => {}) expect(screen.queryByTestId('selected-model')).toBeNull() + expect(screen.queryByText('Loading models...')).toBeNull() + fireEvent.change(screen.getByRole('combobox', { name: 'Select a model' }), { + target: { value: 'lower' }, + }) + await selected('lower') + }) + + it('offers model selection when the active-file lookup fails', async () => { + rpc.mockRejectedValue(new Error('RPC unavailable')) + render(page()) + await screen.findByText( + 'Unable to determine the active model. Select a model to view its lineage.', + ) + expect(screen.queryByText('Loading models...')).toBeNull() + fireEvent.change(screen.getByRole('combobox', { name: 'Select a model' }), { + target: { value: 'second' }, + }) + await selected('second') + }) + + it('ignores a rejected lookup after a newer focus selects a model', async () => { + let reject!: (reason: Error) => void + rpc.mockReturnValue( + new Promise((_, fail) => { + reject = fail + }), + ) + render(page()) + focus('c:/project/SECOND.SQL', true) + await selected('second') + await act(async () => { + reject(new Error('RPC unavailable')) + }) + await selected('second') + expect(screen.queryByRole('combobox')).toBeNull() + }) + + it('keeps a manual selection when a pending initial lookup finishes', async () => { + let resolve!: (value: { fileUri: string; isWindows: boolean }) => void + rpc.mockReturnValue( + new Promise(done => { + resolve = done + }), + ) + render(page()) + fireEvent.change(screen.getByRole('combobox', { name: 'Select a model' }), { + target: { value: 'second' }, + }) + await selected('second') + await act(async () => { + resolve({ fileUri: models[0].full_path, isWindows: true }) + }) + await selected('second') }) it.each([undefined, 'C:/project/README.md'])( @@ -161,6 +221,7 @@ describe('lineage active-editor selection', () => { api.mockReturnValue({ data: [], isLoading: false }) const view = render(page()) expect(rpc).not.toHaveBeenCalled() + expect(screen.getByText('No models available.')).toBeTruthy() expect(screen.queryByTestId('selected-model')).toBeNull() api.mockReturnValue({ data: models, isLoading: false }) view.rerender(page()) diff --git a/vscode/react/src/pages/lineage.tsx b/vscode/react/src/pages/lineage.tsx index 6047ed2c4c..3d37f7751f 100644 --- a/vscode/react/src/pages/lineage.tsx +++ b/vscode/react/src/pages/lineage.tsx @@ -75,10 +75,12 @@ export function LineagePage() { ) } +/** Follow editor focus without choosing arbitrarily between ambiguous model paths. */ function Lineage() { const [selectedModel, setSelectedModel] = useState( undefined, ) + const [activeFileError, setActiveFileError] = useState(false) const { on } = useEventBus() const queryClient = useQueryClient() @@ -100,21 +102,27 @@ function Lineage() { let current = true const version = focusVersion.current // get_active_file uses a plain payload on the existing RPC wire protocol. - rpc('get_active_file', {}).then(response => { - if (!current || focusVersion.current !== version) return - const activeFile = - response as unknown as RPCMethods['get_active_file']['result'] - const matches = findModelsByPath( - models, - activeFile.fileUri, - activeFile.isWindows, - ) - setSelectedModel( - previous => - previous ?? - (matches.length > 1 ? undefined : (matches[0] ?? models[0]).name), - ) - }) + setActiveFileError(false) + rpc('get_active_file', {}) + .then(response => { + if (!current || focusVersion.current !== version) return + const activeFile = + response as unknown as RPCMethods['get_active_file']['result'] + const matches = findModelsByPath( + models, + activeFile.fileUri, + activeFile.isWindows, + ) + setSelectedModel( + previous => + previous ?? + (matches.length > 1 ? undefined : (matches[0] ?? models[0]).name), + ) + }) + .catch(() => { + if (!current || focusVersion.current !== version) return + setActiveFileError(true) + }) return () => { current = false } @@ -172,18 +180,48 @@ function Lineage() { return
Error: {modelsError.message}
} - if ( - isLoadingModels || - models === undefined || - modelsRecord === false || - selectedModel === undefined - ) { + if (isLoadingModels || models === undefined || modelsRecord === false) { return
Loading models...
} if (!Array.isArray(models)) { return
Error: Models data is not in the expected format
} + if (!models.length) { + return
No models available.
+ } + if (selectedModel === undefined) { + return ( +
+

+ {activeFileError + ? 'Unable to determine the active model. Select a model to view its lineage.' + : 'Select a model to view its lineage.'} +

+ +
+ ) + } + return (