From 649ed87a6de0ccfa8176d2c799d889996515bb30 Mon Sep 17 00:00:00 2001 From: Mick Cooney Date: Sat, 17 Oct 2026 18:42:18 +0100 Subject: [PATCH] fix(r): read contains= case-sensitively, and pin the VIRTUAL heritage edge R matches argument names case-sensitively, so setClass("A", CONTAINS = "B") names no base; the scope query linked it anyway. Drop the CONTAINS spelling from both predicates. Pin the VIRTUAL placeholder: a class naming both a real base and VIRTUAL gets both edges (the dedup set must be seeded only from edges that target the placeholder), the edge keeps its confidence and reason, and running the heritage hook twice mints one node and one edge. --- .../src/core/ingestion/languages/r/query.ts | 4 +- .../r-s4-contains-forms/R/classes.R | 10 +++ .../resolvers/r-virtual-heritage.test.ts | 69 ++++++++++++++++++- 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/r-s4-contains-forms/R/classes.R diff --git a/gitnexus/src/core/ingestion/languages/r/query.ts b/gitnexus/src/core/ingestion/languages/r/query.ts index 3467706cc..edec91a8a 100644 --- a/gitnexus/src/core/ingestion/languages/r/query.ts +++ b/gitnexus/src/core/ingestion/languages/r/query.ts @@ -303,7 +303,7 @@ export const R_SCOPE_QUERY = ` arguments: (arguments (argument name: (identifier) @_contains1 - (#match? @_contains1 "^(contains|CONTAINS)$") + (#match? @_contains1 "^(contains)$") value: (string content: (string_content) @reference.inherits)))) @@ -315,7 +315,7 @@ export const R_SCOPE_QUERY = ` arguments: (arguments (argument name: (identifier) @_contains2 - (#match? @_contains2 "^(contains|CONTAINS)$") + (#match? @_contains2 "^(contains)$") value: (call function: (identifier) @_cfn2 (#match? @_cfn2 "^c$") diff --git a/gitnexus/test/fixtures/lang-resolution/r-s4-contains-forms/R/classes.R b/gitnexus/test/fixtures/lang-resolution/r-s4-contains-forms/R/classes.R new file mode 100644 index 000000000..c292bf8ca --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/r-s4-contains-forms/R/classes.R @@ -0,0 +1,10 @@ +# `contains` is matched case-sensitively, as R matches argument names: `CONTAINS =` links nothing. +setClass("Base", representation(x = "numeric")) + +# Both a real base and the VIRTUAL sentinel: each gets its own EXTENDS edge. +setClass("Both", contains = c("Base", "VIRTUAL")) + +# The sentinel written first, then a real base. +setClass("SentinelFirst", contains = c("VIRTUAL", "Base")) + +setClass("Shouted", CONTAINS = "Base") diff --git a/gitnexus/test/integration/resolvers/r-virtual-heritage.test.ts b/gitnexus/test/integration/resolvers/r-virtual-heritage.test.ts index 2f3874f02..fe54c3957 100644 --- a/gitnexus/test/integration/resolvers/r-virtual-heritage.test.ts +++ b/gitnexus/test/integration/resolvers/r-virtual-heritage.test.ts @@ -7,7 +7,8 @@ */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import path from 'path'; -import { FIXTURES, getRelationships, runPipelineFromRepo } from './helpers.js'; +import { FIXTURES, getNodesByLabel, getRelationships, runPipelineFromRepo } from './helpers.js'; +import { rScopeResolver } from '../../../src/core/ingestion/languages/r/scope-resolver.js'; import type { PipelineResult } from './helpers.js'; beforeAll(() => vi.stubEnv('GITNEXUS_WORKER_READY_TIMEOUT_MS', '60000')); @@ -52,3 +53,69 @@ describe('R: the S4 sentinel and an R6 base of the same name in one repository', expect(extendsEdges(result, 'Child')).toEqual([]); }); }); + +describe('R: the VIRTUAL placeholder edge next to other heritage edges', () => { + let result: PipelineResult; + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'r-s4-contains-forms'), () => {}); + }, 60000); + + it.each(['Both', 'SentinelFirst'])( + '%s gets an edge to its real base and one to the placeholder', + (source) => { + expect( + extendsEdges(result, source) + .map((e) => e.rel.targetId) + .sort(), + ).toEqual([PLACEHOLDER, 'Class:R/classes.R:Base'].sort()); + }, + ); + + it('gives the placeholder edge the confidence and reason of a resolved virtual heritage edge', () => { + const edge = extendsEdges(result, 'Both').find((e) => e.rel.targetId === PLACEHOLDER); + expect(edge?.rel.confidence).toBe(0.85); + expect(edge?.rel.reason).toBe('scope-resolution: inherits (virtual)'); + }); + + it('mints exactly one placeholder node', () => { + expect(result.graph.getNode(PLACEHOLDER)?.label).toBe('Class'); + expect(getNodesByLabel(result, 'Class').filter((n) => n === 'VIRTUAL')).toHaveLength(1); + }); + + it('reads `contains` case-sensitively: CONTAINS = "Base" links nothing', () => { + expect(extendsEdges(result, 'Shouted')).toEqual([]); + }); +}); + +describe('R: emitting the VIRTUAL heritage edges twice changes nothing', () => { + let result: PipelineResult; + let calls = 0; + beforeAll(async () => { + const original = rScopeResolver.emitHeritageEdges; + if (original === undefined) throw new Error('the R scope resolver emits heritage edges'); + const spy = vi + .spyOn(rScopeResolver, 'emitHeritageEdges') + .mockImplementation((...args: Parameters) => { + calls++; + original(...args); + original(...args); + }); + try { + result = await runPipelineFromRepo(path.join(FIXTURES, 'r-s4-contains-forms'), () => {}); + } finally { + spy.mockRestore(); + } + }, 60000); + + it('ran the doubled hook (otherwise the checks below prove nothing)', () => { + expect(calls).toBeGreaterThan(0); + }); + + it('keeps one placeholder node and one edge per class that names it', () => { + expect(getNodesByLabel(result, 'Class').filter((n) => n === 'VIRTUAL')).toHaveLength(1); + const toPlaceholder = getRelationships(result, 'EXTENDS').filter( + (e) => e.rel.targetId === PLACEHOLDER, + ); + expect(toPlaceholder.map((e) => e.source).sort()).toEqual(['Both', 'SentinelFirst']); + }); +});