fix(review): keep notebook JSON coordinates and extract once

Review findings: last-wins cells arrays, source-string line anchors, skip cells without source, reuse worker extraction for scope captures, and fail closed in embeddings.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Gergo Magyar 2026-09-25 16:31:16 +00:00
parent fc4b48d1f1
commit 5535df854d
12 changed files with 247 additions and 56 deletions

View file

@ -11,7 +11,7 @@ import {
} from '../tree-sitter/parser-loader.js';
import { parseSourceSafe } from '../tree-sitter/safe-parse.js';
import { getLanguageForFileContent, getProvider } from '../ingestion/languages/index.js';
import { extractNotebookPython } from '../ingestion/ipynb-extractor.js';
import { extractNotebookPython, isNotebookPath } from '../ingestion/ipynb-extractor.js';
const parserCache = new Map<string, any>();
@ -42,13 +42,12 @@ export const ensureAndParse = async (content: string, filePath: string): Promise
// offsets still index `content` except for `.ipynb`, which is replaced by
// concatenated code-cell Python (same as the parse worker).
let parseContent = getProvider(language).preprocessSource?.(content, filePath) ?? content;
if (filePath.replace(/\\/g, '/').toLowerCase().endsWith('.ipynb')) {
if (isNotebookPath(filePath)) {
const extracted = extractNotebookPython(content);
if (extracted) {
parseContent =
getProvider(language).preprocessSource?.(extracted.pythonSource, filePath) ??
extracted.pythonSource;
}
if (!extracted) return null;
parseContent =
getProvider(language).preprocessSource?.(extracted.pythonSource, filePath) ??
extracted.pythonSource;
}
return parseSourceSafe(parserInstance, parseContent);

View file

@ -2,7 +2,8 @@
* Jupyter notebook (.ipynb) Python extractor.
*
* Pulls code-cell source from nbformat JSON so the Python tree-sitter
* grammar can parse it. Pure — no I/O, no tree-sitter, worker-safe.
* grammar can parse it. Extraction helpers are I/O-free and worker-safe.
* `extractNotebookPythonCached` is an optional process-local LRU for FTS.
*
* Graph coordinates stay 0-based lines in the on-disk JSON file. Extract
* buffer lines map through {@link mapExtractLine}.
@ -22,6 +23,10 @@ export interface NotebookPythonExtraction {
const PYTHON_FAMILY = new Set(['python', 'python2', 'python3', 'ipython']);
export function isNotebookPath(filePath: string): boolean {
return filePath.replace(/\\/g, '/').toLowerCase().endsWith('.ipynb');
}
export function isPythonFamilyLanguage(name: string | undefined | null): boolean {
if (name === undefined || name === null) return false;
const n = name.trim().toLowerCase();
@ -29,13 +34,29 @@ export function isPythonFamilyLanguage(name: string | undefined | null): boolean
return /^python\d/.test(n);
}
function indexToLine(content: string, index: number): number {
let line = 0;
const end = Math.max(0, Math.min(index, content.length));
for (let i = 0; i < end; i++) {
if (content.charCodeAt(i) === 10) line++;
function buildLineStarts(content: string): number[] {
const starts = [0];
for (let i = 0; i < content.length; i++) {
if (content.charCodeAt(i) === 10) starts.push(i + 1);
}
return line;
return starts;
}
function indexToLine(lineStarts: readonly number[], index: number): number {
if (index <= 0) return 0;
let lo = 0;
let hi = lineStarts.length - 1;
let ans = 0;
while (lo <= hi) {
const mid = (lo + hi) >> 1;
if (lineStarts[mid] <= index) {
ans = mid;
lo = mid + 1;
} else {
hi = mid - 1;
}
}
return ans;
}
function skipWs(content: string, i: number): number {
@ -144,26 +165,88 @@ function findDepth1Key(content: string, objStart: number, objEnd: number, key: s
return -1;
}
function findLastDepth1Key(content: string, objStart: number, objEnd: number, key: string): number {
let depth = 0;
let inStr = false;
let j = objStart;
let found = -1;
while (j < objEnd) {
const ch = content[j];
if (inStr) {
if (ch === '\\') j += 2;
else {
if (ch === '"') inStr = false;
j++;
}
continue;
}
if (ch === '"') {
if (depth === 1 && content.startsWith(key, j)) found = j;
inStr = true;
j++;
continue;
}
if (ch === '{' || ch === '[') depth++;
else if (ch === '}' || ch === ']') depth--;
j++;
}
return found;
}
function findCellsArraySpan(content: string): { start: number; end: number } | null {
const root = jsonBraceSpan(content, 0, '{', '}');
if (!root) return null;
const cellsKey = findDepth1Key(content, root.start, root.end, '"cells"');
const cellsKey = findLastDepth1Key(content, root.start, root.end, '"cells"');
if (cellsKey < 0) return null;
const colon = content.indexOf(':', cellsKey + 7);
if (colon < 0 || colon >= root.end) return null;
return jsonBraceSpan(content, colon + 1, '[', ']');
}
function nextUnquotedChar(content: string, from: number, until: number, needle: '{' | '"'): number {
let inStr = false;
let j = from;
while (j < until) {
const ch = content[j];
if (inStr) {
if (ch === '\\') j += 2;
else {
if (ch === '"') inStr = false;
j++;
}
continue;
}
if (ch === '"') {
if (needle === '"') return j;
inStr = true;
j++;
continue;
}
if (ch === needle) return j;
j++;
}
return -1;
}
function firstQuoteInValue(content: string, span: { start: number; end: number }): number {
const i = skipWs(content, span.start);
if (i < span.end && content[i] === '"') return i;
if (i < span.end && content[i] === '[') {
const q = nextUnquotedChar(content, i + 1, span.end, '"');
if (q >= 0) return q;
}
return span.start;
}
function findNextCodeCellSourceSpan(
content: string,
from: number,
): { span: { start: number; end: number }; nextFrom: number } | null {
const cells = findCellsArraySpan(content);
if (!cells) return null;
cells: { start: number; end: number },
): { span: { start: number; end: number } | null; nextFrom: number } | null {
let search = Math.max(from, cells.start + 1);
while (search < cells.end) {
const brace = content.indexOf('{', search);
if (brace < 0 || brace >= cells.end) return null;
const brace = nextUnquotedChar(content, search, cells.end, '{');
if (brace < 0) return null;
const obj = jsonBraceSpan(content, brace, '{', '}');
if (!obj || obj.end > cells.end) {
search = brace + 1;
@ -186,18 +269,15 @@ function findNextCodeCellSourceSpan(
}
const sourceKey = findDepth1Key(content, obj.start, obj.end, '"source"');
if (sourceKey < 0) {
search = obj.end;
continue;
return { span: null, nextFrom: obj.end };
}
const srcColon = content.indexOf(':', sourceKey + 8);
if (srcColon < 0 || srcColon >= obj.end) {
search = obj.end;
continue;
return { span: null, nextFrom: obj.end };
}
const span = jsonValueSpan(content, srcColon + 1);
if (!span) {
search = obj.end;
continue;
return { span: null, nextFrom: obj.end };
}
return { span, nextFrom: obj.end };
}
@ -287,6 +367,10 @@ export function extractNotebookPython(content: string): NotebookPythonExtraction
if (!Array.isArray(nb.cells)) return null;
if (kernelShouldSkip(nb)) return null;
const cells = findCellsArraySpan(content);
if (!cells) return null;
const lineStarts = buildLineStarts(content);
const chunks: string[] = [];
const segments: NotebookLineSegment[] = [];
let searchFrom = 0;
@ -296,9 +380,10 @@ export function extractNotebookPython(content: string): NotebookPythonExtraction
const cell = rawCell as Record<string, unknown>;
if (cell.cell_type !== 'code') continue;
const located = findNextCodeCellSourceSpan(content, searchFrom);
const located = findNextCodeCellSourceSpan(content, searchFrom, cells);
if (!located) return null;
searchFrom = located.nextFrom;
if (!located.span) continue;
const lang = cellLanguage(cell);
if (lang !== undefined && !isPythonFamilyLanguage(lang)) {
@ -306,8 +391,8 @@ export function extractNotebookPython(content: string): NotebookPythonExtraction
}
const { skipCell, lines } = processCellLines(flattenSource(cell.source));
const jsonStartLine = indexToLine(content, located.span.start);
const jsonEndLine = Math.max(jsonStartLine, indexToLine(content, located.span.end - 1));
const jsonStartLine = indexToLine(lineStarts, firstQuoteInValue(content, located.span));
const jsonEndLine = Math.max(jsonStartLine, indexToLine(lineStarts, located.span.end - 1));
if (skipCell) {
continue;
@ -385,6 +470,7 @@ export function notebookPythonSnippetFromExtract(
const pyLines = extracted.pythonSource.split('\n');
const out: string[] = [];
for (const seg of extracted.segments) {
if (seg.jsonEndLine < startLine || seg.jsonStartLine > endLine) continue;
for (let extract = seg.extractStartLine; extract <= seg.extractEndLine; extract++) {
const jsonLine = mapExtractLine(extract, extracted.segments);
if (jsonLine >= startLine && jsonLine <= endLine) {

View file

@ -828,6 +828,13 @@ interface LanguageProviderConfig {
*/
sourceMeta?: {
readonly sourceKind?: 'full-file' | 'pre-extracted-script';
/** Python `.ipynb` only: JSON line segments for the pre-extracted buffer. */
readonly notebookSegments?: readonly {
readonly extractStartLine: number;
readonly extractEndLine: number;
readonly jsonStartLine: number;
readonly jsonEndLine: number;
}[];
},
) => readonly CaptureMatch[];

View file

@ -26,6 +26,7 @@ import { splitImportStatement } from './import-decomposer.js';
import { getPythonParser, getPythonScopeQuery } from './query.js';
import {
extractNotebookPython,
isNotebookPath,
mapExtractLine,
type NotebookLineSegment,
} from '../../ipynb-extractor.js';
@ -64,20 +65,27 @@ export function emitPythonScopeCaptures(
sourceText: string,
filePath: string,
cachedTree?: unknown,
sourceMeta?: { sourceKind?: 'full-file' | 'pre-extracted-script' },
sourceMeta?: {
sourceKind?: 'full-file' | 'pre-extracted-script';
notebookSegments?: readonly NotebookLineSegment[];
},
): readonly CaptureMatch[] {
let parseText = sourceText;
let tree = cachedTree as ReturnType<ReturnType<typeof getPythonParser>['parse']> | undefined;
let notebookSegments: readonly NotebookLineSegment[] | undefined;
if (filePath.replace(/\\/g, '/').toLowerCase().endsWith('.ipynb')) {
const extracted = extractNotebookPython(sourceText);
if (extracted === null) {
if (sourceMeta?.sourceKind !== 'pre-extracted-script') return [];
if (isNotebookPath(filePath)) {
if (sourceMeta?.notebookSegments) {
notebookSegments = sourceMeta.notebookSegments;
} else {
parseText = extracted.pythonSource;
notebookSegments = extracted.segments;
if (sourceMeta?.sourceKind !== 'pre-extracted-script') {
tree = undefined;
const extracted = extractNotebookPython(sourceText);
if (extracted === null) {
if (sourceMeta?.sourceKind !== 'pre-extracted-script') return [];
} else {
parseText = extracted.pythonSource;
notebookSegments = extracted.segments;
if (sourceMeta?.sourceKind !== 'pre-extracted-script') {
tree = undefined;
}
}
}
}

View file

@ -45,6 +45,12 @@ export function extractParsedFile(
onWarn?: ScopeBridgeWarn,
cachedTree?: unknown,
sourceKind: ScopeCaptureSourceKind = 'full-file',
notebookSegments?: readonly {
readonly extractStartLine: number;
readonly extractEndLine: number;
readonly jsonStartLine: number;
readonly jsonEndLine: number;
}[],
): ParsedFile | undefined {
if (provider.emitScopeCaptures === undefined) return undefined;
if (sourceText.trim().length === 0) return undefined;
@ -58,7 +64,10 @@ export function extractParsedFile(
cachedTree === undefined
? (provider.preprocessSource?.(sourceText, filePath) ?? sourceText)
: sourceText;
const captures = provider.emitScopeCaptures(parseText, filePath, cachedTree, { sourceKind });
const captures = provider.emitScopeCaptures(parseText, filePath, cachedTree, {
sourceKind,
...(notebookSegments ? { notebookSegments } : {}),
});
return extractScope(captures, filePath, provider);
} catch (err) {
const message = `scope extraction failed for ${filePath}: ${

View file

@ -137,6 +137,7 @@ import {
} from '../vue-sfc-extractor.js';
import {
extractNotebookPython,
isNotebookPath,
mapExtractLine,
type NotebookLineSegment,
} from '../ipynb-extractor.js';
@ -1612,10 +1613,7 @@ const processFileGroup = (
scopeSourceKind = 'pre-extracted-script';
lineOffset = extracted.lineOffset;
isVueSetup = extracted.isSetup;
} else if (
language === SupportedLanguages.Python &&
file.path.replace(/\\/g, '/').toLowerCase().endsWith('.ipynb')
) {
} else if (language === SupportedLanguages.Python && isNotebookPath(file.path)) {
const extracted = extractNotebookPython(file.content);
if (!extracted) continue;
parseContent = extracted.pythonSource;
@ -1685,14 +1683,15 @@ const processFileGroup = (
let scopeExtractionFailed = false;
const parsedFile = extractParsedFile(
provider,
notebookSegments ? file.content : parseContent,
parseContent,
file.path,
(message) => {
scopeExtractionFailed = true;
reportWarning(message);
},
tree,
notebookSegments ? 'full-file' : scopeSourceKind,
scopeSourceKind,
notebookSegments,
);
if (scopeExtractionFailed) (result.scopeExtractionFailures ??= []).push(file.path);
if (parsedFile !== undefined) {
@ -1931,7 +1930,7 @@ const processFileGroup = (
filePath: file.path,
httpMethod,
decoratorName,
lineNumber: decoratorNode.startPosition.row + lineOffset,
lineNumber: mapRow(decoratorNode.startPosition.row),
...(decoratorReceiver ? { decoratorReceiver } : {}),
...(handlerName ? { handlerName } : {}),
};

View file

@ -24,8 +24,9 @@ import { parseTruthyEnv } from '../ingestion/utils/env.js';
import { SYMBOL_NODE_LABELS } from '../ingestion/utils/symbol-labels.js';
import { applyCjkSegmentationIfEnabled } from '../search/cjk-segmentation.js';
import {
notebookPythonSnippetFromExtract,
extractNotebookPythonCached,
isNotebookPath,
notebookPythonSnippetFromExtract,
} from '../ingestion/ipynb-extractor.js';
/** Computed once — `RELATION_SCHEMA` is a static template literal. Exported so
@ -329,10 +330,8 @@ const extractContent = async (
const endLine = node.properties.endLine;
if (startLine === undefined || endLine === undefined) return '';
const notebookPath = String(filePath ?? '')
.replace(/\\/g, '/')
.toLowerCase();
if (notebookPath.endsWith('.ipynb')) {
const notebookPath = String(filePath ?? '');
if (isNotebookPath(notebookPath)) {
const extracted = extractNotebookPythonCached(notebookPath, content);
const reconstructed = extracted
? notebookPythonSnippetFromExtract(extracted, startLine, endLine)

View file

@ -778,9 +778,6 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid
// v104 (#3339 review): TS/JS pair-HOC queries now name object-pair
// `mutation(withAuth(arrow))` handlers. Warm caches replay the pre-fix
// capture set (anonymous arrows, no Function name), so both stores re-extract.
// v104 (#3339 review): TS/JS pair-HOC queries now name object-pair
// `mutation(withAuth(arrow))` handlers. Warm caches replay the pre-fix
// capture set (anonymous arrows, no Function name), so both stores re-extract.
// v105 (#3371): `.ipynb` code cells are extracted to Python before parse.
// Warm caches keyed on raw JSON would replay empty/failed Python parses.
const SCHEMA_BUMP = 105;

View file

@ -12,6 +12,7 @@ import {
runPipelineFromRepo,
writeFixtureRepo,
} from './helpers.js';
import { extractNotebookPython } from '../../../src/core/ingestion/ipynb-extractor.js';
function pythonNotebook(cells: Array<{ source: string | string[]; language?: string }>): string {
return JSON.stringify(
@ -62,7 +63,10 @@ describe('Jupyter notebook Python pipeline', () => {
expect(functions).toContain('helper');
const train = getNodesByLabelFull(result, 'Function').find((n) => n.name === 'train');
expect(train?.properties.filePath.replace(/\\/g, '/')).toMatch(/analysis\.ipynb$/);
expect(train?.properties.startLine).toBeGreaterThan(0);
const extracted = extractNotebookPython(
fs.readFileSync(path.join(root, 'analysis.ipynb'), 'utf8'),
)!;
expect(train?.properties.startLine).toBe(extracted.segments[1].jsonStartLine);
const imports = getRelationships(result, 'IMPORTS');
expect(
imports.some(

View file

@ -123,4 +123,33 @@ describe('ensureAndParse', () => {
expect(objcParse).toHaveBeenCalledTimes(2);
expect(cppParse).toHaveBeenCalledTimes(1);
});
it('parses extracted notebook Python and returns null when extraction fails', async () => {
parseSourceSafeSpy.mockClear();
const pyParse = vi.fn().mockReturnValue({ lang: 'py', rootNode: { type: 'module' } });
createParserForLanguage.mockImplementation(async (language: string) => {
if (language === 'python') return { parse: pyParse };
throw new Error(`unexpected language ${language}`);
});
const { ensureAndParse } = await import('../../src/core/embeddings/ast-utils.js');
const nb = JSON.stringify({
nbformat: 4,
nbformat_minor: 5,
metadata: { kernelspec: { language: 'python', name: 'python3', display_name: 'Python' } },
cells: [
{ cell_type: 'code', metadata: {}, source: ['def train():\n', ' pass\n'], outputs: [] },
],
});
await ensureAndParse(nb, 'analysis.ipynb');
expect(parseSourceSafeSpy).toHaveBeenCalled();
const parsedText = parseSourceSafeSpy.mock.calls.at(-1)?.[1] as string;
expect(parsedText).toContain('def train');
expect(parsedText).not.toContain('cell_type');
parseSourceSafeSpy.mockClear();
expect(await ensureAndParse('{not json', 'broken.ipynb')).toBeNull();
expect(parseSourceSafeSpy).not.toHaveBeenCalled();
});
});

View file

@ -75,6 +75,8 @@ describe('extractNotebookPython', () => {
expect(result).not.toBeNull();
expect(result!.pythonSource).toContain('def train():');
expect(result!.segments).toHaveLength(1);
const defJsonLine = content.split('\n').findIndex((l) => l.includes('def train'));
expect(result!.segments[0].jsonStartLine).toBe(defJsonLine);
});
it('keeps two code cells in order with two segments', () => {
@ -237,3 +239,52 @@ describe('notebookPythonSnippet', () => {
expect(snippet).not.toContain('cell_type');
});
});
describe('extractNotebookPython edge cases', () => {
it('skips a code cell without source and keeps later Python', () => {
const content = notebook({
language: 'python',
cells: [
{ cell_type: 'code', metadata: {}, source: ['def ok():\n', ' pass\n'], outputs: [] },
{ cell_type: 'code', metadata: {}, outputs: [] },
{ cell_type: 'code', metadata: {}, source: ['def train():\n', ' pass\n'], outputs: [] },
],
});
const result = extractNotebookPython(content);
expect(result!.pythonSource).toContain('def ok');
expect(result!.pythonSource).toContain('def train');
});
it('returns null when language_info is julia without kernelspec', () => {
const content = notebook({
languageInfo: 'julia',
cells: [{ cell_type: 'code', metadata: {}, source: ['1 + 1\n'], outputs: [] }],
});
expect(extractNotebookPython(content)).toBeNull();
});
it('maps coordinates to the last cells array when the key is duplicated', () => {
const decoy = JSON.stringify(
[{ cell_type: 'code', metadata: {}, source: ['def decoy():\n', ' pass\n'], outputs: [] }],
null,
2,
);
const real = JSON.stringify(
[{ cell_type: 'code', metadata: {}, source: ['def real():\n', ' pass\n'], outputs: [] }],
null,
2,
);
const content = `{
"nbformat": 4,
"nbformat_minor": 5,
"metadata": { "kernelspec": { "language": "python", "name": "python3", "display_name": "Python" } },
"cells": ${decoy},
"cells": ${real}
}`;
const result = extractNotebookPython(content);
expect(result!.pythonSource).toContain('def real');
expect(result!.pythonSource).not.toContain('decoy');
const realLine = content.split('\n').findIndex((l) => l.includes('def real'));
expect(result!.segments[0].jsonStartLine).toBe(realLine);
});
});

View file

@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest';
import { emitPythonScopeCaptures } from '../../src/core/ingestion/languages/python/captures.js';
import { pythonProvider } from '../../src/core/ingestion/languages/python.js';
import { extractParsedFile } from '../../src/core/ingestion/scope-extractor-bridge.js';
import { extractNotebookPython, mapExtractLine } from '../../src/core/ingestion/ipynb-extractor.js';
import { getLanguageFromFilename, SupportedLanguages } from 'gitnexus-shared';
import { getProviderForFile } from '../../src/core/ingestion/languages/index.js';
@ -39,7 +40,9 @@ describe('Python notebook scope captures', () => {
it('extractParsedFile yields a Function for train', () => {
const captured = emitPythonScopeCaptures(notebook, 'analysis.ipynb');
const fnCapture = captured.find((m) => m['@scope.function'] !== undefined);
expect(fnCapture?.['@scope.function']?.range.startLine).toBeGreaterThan(1);
const extracted = extractNotebookPython(notebook)!;
const expectedJson = mapExtractLine(extracted.segments[0].extractStartLine, extracted.segments);
expect(fnCapture?.['@scope.function']?.range.startLine).toBe(expectedJson + 1);
const parsed = extractParsedFile(pythonProvider, notebook, 'analysis.ipynb');
expect(parsed).toBeDefined();
expect(parsed!.localDefs.some((d) => d.qualifiedName === 'train' || d.name === 'train')).toBe(