mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(scope-resolution): close every deferred item on #2699 (A1 values, twin-list guard, schema bumps)
Clears the limitations this PR had been carrying rather than leaving them as follow-ups. ## A1 — function-local VALUES now carry their own identity This was #2699's ORIGINAL complaint and the one a callable-only gate could never reach: a top-level `const handler` and a function-local `const handler` collapsed onto ONE `Const:v.ts:handler`. #2695 restricted position-qualified identity to Function|Method|Constructor because the collision that produced wrong CALLS edges was between callables, and widening churned ids for symbols the pruner mostly deletes. The churn is real and is accepted here deliberately. Widening needed THREE gates aligned, not one: - id-building — `parse-worker.ts` nestedCallablePrefix - resolution — `ids.ts` position key - registration — `node-lookup.ts` position-key registration Missing the third would register no position key for values, so every lookup misses and falls through silently. That is the #2714 failure mode: the caller attaches to a node that does not exist and the edge is DROPPED, which looks like "zero dangling edges" from outside. All three now route through ONE predicate, `isPositionQualifiedLocalLabel`, rather than repeating the label set a third time. Only LOCALS move. The prefix comes from `enclosingCallablePrefix`, which returns undefined when nothing encloses the declaration, so top-level and class-member ids are untouched — verified by the full resolver sweep, where a leak onto class members would have broken assertions in every language. `Property` is included on purpose: a class field stays unqualified because the prefix walk boundaries on class-likes, while an object-literal property inside a function is genuinely local and would otherwise keep the old collision. Measured: `Const:v.ts:handler` + `Const:v.ts:run.handler@3:2`, two distinct nodes. The KNOWN LIMIT test is FLIPPED per its own former instruction ("this test should be updated as part of it rather than deleted"). ## Schema bumps — required by Part B, not just by A1 INCREMENTAL_SCHEMA_VERSION 20 -> 21, parse-cache SCHEMA_BUMP 27 -> 29. SCHEMA_BUMP is 29, not 28, and that is the point of re-checking it against origin/main at MERGE time rather than branch time. This branch cut at 27 and bumped to 28; #2415 also bumped 27 -> 28 and merged first. The automated main-merge onto this branch surfaced the collision — leaving it at 28 would have shipped this whole change with NO parse-cache invalidation, so every warm cache keeps replaying the pre-fix captures and ids. This is the third instance of that collision recorded in parse-cache.ts (#2632/#2653 hit it at v21, and #2653/#2654 hit INCREMENTAL_SCHEMA_VERSION the same way). Part B already changed emitted node ids AND edges on files that did not themselves change (Dart locals re-keyed, Rust gained a node it never emitted, five languages gained closure-source attribution). A v20 index topped up incrementally keeps serving the old attribution, and a warm parse cache replays the old captures and ids verbatim. Shipping S1-S4 without these would have let every existing index silently keep the pre-fix graph. ## Twin-list drift guard — the sixth instance in this family `IMPLICIT_RECEIVERS` (gitnexus-shared lookup-core.ts) and `THIS_RECEIVERS` (type-env.ts) spell the same concept in two packages, and nothing enforced agreement — `$this` was added to the shared list in #2714 only because it was already in the other. New structural test asserts set equality plus the ONE deliberate asymmetry (`Me`, Visual Basic spelling, absent from the shared list because no SupportedLanguages entry uses it) in BOTH directions, so re-adding it there or dropping it here each fail loudly. Structural rather than value-imported: both constants are module-private, and exporting them purely to be testable would widen two public surfaces to satisfy a test. ## Two false comments corrected - `lookup-core.ts` said "see the drift guard noted in #2714", implying a guard existed when it was only a deferred follow-up. It exists now, and the comment points at it. - `callable-id-lockstep.test.ts` claimed its regex "fails if any site reconstructs the id". It matches ONE template spelling; a hand-rolled concatenation still slips past. Now stated as a tripwire for the known shape, not a proof. ## Skill learnings Four entries appended to eval/workflow_bench/learnings.jsonl from this run: the v9fs safe-writer failure, backticks silently terminating a query template literal (hit three times), a module-level TDZ const that passes tsc and then presents as N file failures with ZERO failing assertions, and concurrent vitest runs starving worker startup so a whole suite fails at ~5001ms. ## Verification Full resolver sweep 2926 passed / 1 skipped / 0 failed (51 files) — identical to pre-A1, which is the evidence that only locals moved. All EIGHT bench gates PASS with fingerprints UNCHANGED, so no regeneration was needed. function-local-identity, callable-id-lockstep, receiver-twin-list-drift and closure-binding-labels 71/71. tsc --noEmit clean. detect_changes {staged}: 9 changed symbols, 14 affected processes, risk HIGH — expected, and the reason the sweep above is the gate rather than a targeted list. Every affected process routes through `resolveDefGraphId`, the key chain Part A measured at CRITICAL with 23 direct dependents. Deliberately NOT done: the SCIP end state (opaque `local <id>` plus an explicit enclosure EDGE instead of containment encoded in the id string). It is a design direction, not a limitation of this work, and it is INCOMPATIBLE with A1 — A1 widens chain-encoded identity, that removes chain encoding entirely. Bundling both would re-key every local twice. Written up in the research notes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR
This commit is contained in:
parent
6fd9af17a6
commit
da3d8397d2
11 changed files with 215 additions and 31 deletions
|
|
@ -1,2 +1,6 @@
|
||||||
{"skill": "gitnexus-work", "date": "2026-07-25", "task": "#2687 const-arrow Const/Function twin fix in parse-worker + MCP impact envelope", "friction": "Phase 2's Build-current/index-current procedure indexes the repo-under-test, which makes CLI-spawning suites (skip-git-cli, cli/tool-no-index-stderr) time out because repo resolution then opens the 237k-node index from that cwd; they pass at the same commit in an unindexed worktree, so the procedure manufactures false regressions in its own final verification.", "suggestion": "Phase 4 should note that CLI-spawn suites can fail solely because the worktree became an indexed repo, and prescribe the A/B check (same commit, unindexed worktree) instead of leaving the executor to conclude a regression."}
|
{"skill": "gitnexus-work", "date": "2026-07-25", "task": "#2687 const-arrow Const/Function twin fix in parse-worker + MCP impact envelope", "friction": "Phase 2's Build-current/index-current procedure indexes the repo-under-test, which makes CLI-spawning suites (skip-git-cli, cli/tool-no-index-stderr) time out because repo resolution then opens the 237k-node index from that cwd; they pass at the same commit in an unindexed worktree, so the procedure manufactures false regressions in its own final verification.", "suggestion": "Phase 4 should note that CLI-spawn suites can fail solely because the worktree became an indexed repo, and prescribe the A/B check (same commit, unindexed worktree) instead of leaving the executor to conclude a regression."}
|
||||||
{"skill": "gitnexus-work", "date": "2026-07-25", "task": "#2687 same run", "friction": "Phase 2 requires top-level `status: up-to-date` before graph queries, but any uncommitted staged edit makes status report `stale` by design, so the gate is unsatisfiable in the stage -> detect_changes -> commit sequence Phase 3 mandates.", "suggestion": "Scope the up-to-date requirement to index.commit == HEAD + empty incompleteReasons + runnerIdentityStatus current, and state that a `stale` top-level status caused solely by uncommitted working-tree edits is expected at the detect_changes gate."}
|
{"skill": "gitnexus-work", "date": "2026-07-25", "task": "#2687 same run", "friction": "Phase 2 requires top-level `status: up-to-date` before graph queries, but any uncommitted staged edit makes status report `stale` by design, so the gate is unsatisfiable in the stage -> detect_changes -> commit sequence Phase 3 mandates.", "suggestion": "Scope the up-to-date requirement to index.commit == HEAD + empty incompleteReasons + runnerIdentityStatus current, and state that a `stale` top-level status caused solely by uncommitted working-tree edits is expected at the detect_changes gate."}
|
||||||
|
{"skill": "gitnexus-plan", "date": "2026-07-28", "task": "#2699 part B — closure binding as a call SOURCE across PHP/Rust/Kotlin/Ruby/Dart", "friction": "The safe plan writer fails closed on a v9fs (9p) worktree because renameat2(RENAME_NOREPLACE) is unsupported, returning EINVAL, so no plan can ever be published there and Phase 2's 'commit the plan document' step is unreachable.", "suggestion": "Detect the EINVAL-on-renameat2 case explicitly and fall back to open(O_EXCL)+write+fsync, which preserves the no-clobber guarantee the flag exists for; failing that, say v9fs is unsupported instead of surfacing a generic write failure."}
|
||||||
|
{"skill": "gitnexus-work", "date": "2026-07-28", "task": "#2699 part B same run", "friction": "Every language query lives in a TypeScript template literal, so a backtick inside a `;;` comment silently terminates it and produces confusing TS1005/TS1128 parse errors far from the real edit. Hit this three separate times in one session.", "suggestion": "Phase 3 should warn that *.query.ts bodies are template literals and backticks in comments are a syntax error, or the repo should add a lint rule; the build catches it but the error location does not point at the comment."}
|
||||||
|
{"skill": "gitnexus-work", "date": "2026-07-28", "task": "#2699 part B same run", "friction": "A module-level `const` derived from another const declared LOWER in the same file passes tsc and builds a clean dist, then throws ReferenceError (temporal dead zone) at import. It presents as N test FILES failing with ZERO failing assertions, which reads like host/infra flake rather than a code defect.", "suggestion": "Phase 3's verification note should call out that file-level failures with zero test failures usually mean a module-load error, and to grep the run output for ReferenceError before blaming the host."}
|
||||||
|
{"skill": "gitnexus-work", "date": "2026-07-28", "task": "#2699 part B same run", "friction": "Two concurrent `vitest run` invocations on this host starve worker-pool startup: every test in both runs fails at ~5001ms against the default GITNEXUS_WORKER_READY_TIMEOUT_MS, which looks exactly like a real regression across the whole suite.", "suggestion": "Phase 3 should state that verification runs must be serial, and that a whole-suite failure at ~5001ms is worker-startup starvation, not signal."}
|
||||||
|
|
|
||||||
|
|
@ -343,7 +343,9 @@ function resolveReceiverOwner(
|
||||||
* That twin also lists `Me`, deliberately NOT mirrored here: no entry in
|
* That twin also lists `Me`, deliberately NOT mirrored here: no entry in
|
||||||
* `SupportedLanguages` uses it, so it can only ever exempt a variable that
|
* `SupportedLanguages` uses it, so it can only ever exempt a variable that
|
||||||
* happens to be called `Me`. The two lists are otherwise the same set, and
|
* happens to be called `Me`. The two lists are otherwise the same set, and
|
||||||
* nothing enforces that — see the drift guard noted in #2714.
|
* that equality — plus the `Me` exemption in both directions — is now ENFORCED
|
||||||
|
* by `gitnexus/test/unit/receiver-twin-list-drift.test.ts`. Editing either list
|
||||||
|
* without the other fails there.
|
||||||
*/
|
*/
|
||||||
const IMPLICIT_RECEIVERS: readonly string[] = Object.freeze(['self', 'this', '$this']);
|
const IMPLICIT_RECEIVERS: readonly string[] = Object.freeze(['self', 'this', '$this']);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -28,7 +28,10 @@ import {
|
||||||
simpleKey,
|
simpleKey,
|
||||||
type GraphNodeLookup,
|
type GraphNodeLookup,
|
||||||
} from '../graph-bridge/node-lookup.js';
|
} from '../graph-bridge/node-lookup.js';
|
||||||
import { isOverloadableCallable } from '../../utils/callable-labels.js';
|
import {
|
||||||
|
isOverloadableCallable,
|
||||||
|
isPositionQualifiedLocalLabel,
|
||||||
|
} from '../../utils/callable-labels.js';
|
||||||
import { templateConstraintsIdTag } from '../../utils/template-arguments.js';
|
import { templateConstraintsIdTag } from '../../utils/template-arguments.js';
|
||||||
import { parameterShapeIdTag } from '../../utils/method-props.js';
|
import { parameterShapeIdTag } from '../../utils/method-props.js';
|
||||||
/**
|
/**
|
||||||
|
|
@ -206,7 +209,7 @@ export function resolveDefGraphId(
|
||||||
// 0-based, def ids 1-based. An `AMBIGUOUS_POSITION` tombstone (two
|
// 0-based, def ids 1-based. An `AMBIGUOUS_POSITION` tombstone (two
|
||||||
// callables on one line) falls through to the name-based keys below.
|
// callables on one line) falls through to the name-based keys below.
|
||||||
const line = defStartLine(def.nodeId);
|
const line = defStartLine(def.nodeId);
|
||||||
if (line !== undefined && isOverloadableCallable(def.type)) {
|
if (line !== undefined && isPositionQualifiedLocalLabel(def.type)) {
|
||||||
const simple = simpleNameOf(qn);
|
const simple = simpleNameOf(qn);
|
||||||
const posHit = nodeLookup.get(positionKey(filePath, def.type, line - 1, simple));
|
const posHit = nodeLookup.get(positionKey(filePath, def.type, line - 1, simple));
|
||||||
if (posHit !== undefined && posHit !== AMBIGUOUS_POSITION) return posHit;
|
if (posHit !== undefined && posHit !== AMBIGUOUS_POSITION) return posHit;
|
||||||
|
|
|
||||||
|
|
@ -20,7 +20,10 @@
|
||||||
|
|
||||||
import type { NodeLabel, ParameterTypeClass } from 'gitnexus-shared';
|
import type { NodeLabel, ParameterTypeClass } from 'gitnexus-shared';
|
||||||
import type { KnowledgeGraph } from '../../../graph/types.js';
|
import type { KnowledgeGraph } from '../../../graph/types.js';
|
||||||
import { isOverloadableCallable } from '../../utils/callable-labels.js';
|
import {
|
||||||
|
isOverloadableCallable,
|
||||||
|
isPositionQualifiedLocalLabel,
|
||||||
|
} from '../../utils/callable-labels.js';
|
||||||
import { templateConstraintsIdTag } from '../../utils/template-arguments.js';
|
import { templateConstraintsIdTag } from '../../utils/template-arguments.js';
|
||||||
import { parameterShapeIdTag } from '../../utils/method-props.js';
|
import { parameterShapeIdTag } from '../../utils/method-props.js';
|
||||||
|
|
||||||
|
|
@ -135,7 +138,7 @@ export function buildGraphNodeLookup(graph: KnowledgeGraph): GraphNodeLookup {
|
||||||
// Position key (#2699) — see `positionKey`. Second write on a key marks it
|
// Position key (#2699) — see `positionKey`. Second write on a key marks it
|
||||||
// ambiguous rather than letting source order decide.
|
// ambiguous rather than letting source order decide.
|
||||||
const startLine = (props as { startLine?: number }).startLine;
|
const startLine = (props as { startLine?: number }).startLine;
|
||||||
if (startLine !== undefined && isOverloadableCallable(node.label)) {
|
if (startLine !== undefined && isPositionQualifiedLocalLabel(node.label)) {
|
||||||
const posK = positionKey(props.filePath, node.label, startLine, props.name);
|
const posK = positionKey(props.filePath, node.label, startLine, props.name);
|
||||||
lookup.set(posK, lookup.has(posK) ? AMBIGUOUS_POSITION : node.id);
|
lookup.set(posK, lookup.has(posK) ? AMBIGUOUS_POSITION : node.id);
|
||||||
// A local-identity node carries `@<row>:<col>` on its last name segment. Record
|
// A local-identity node carries `@<row>:<col>` on its last name segment. Record
|
||||||
|
|
|
||||||
|
|
@ -14,3 +14,37 @@ import type { NodeLabel } from 'gitnexus-shared';
|
||||||
export function isOverloadableCallable(label: NodeLabel | undefined): boolean {
|
export function isOverloadableCallable(label: NodeLabel | undefined): boolean {
|
||||||
return label === 'Function' || label === 'Method' || label === 'Constructor';
|
return label === 'Function' || label === 'Method' || label === 'Constructor';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Labels whose FUNCTION-LOCAL declarations carry the enclosing-callable +
|
||||||
|
* position identity of #2699 (`Function:x.ts:run.save@3:2`).
|
||||||
|
*
|
||||||
|
* Wider than {@link isOverloadableCallable} on purpose. #2695 restricted the
|
||||||
|
* rule to callables because the collision that produced wrong CALLS edges was
|
||||||
|
* between callables, and widening churned ids for symbols the local-symbol
|
||||||
|
* pruner mostly deletes. But the issue's ORIGINAL complaint was about values:
|
||||||
|
* a top-level `const handler` and a function-local `const handler` collapsed
|
||||||
|
* onto one `Const:v.ts:handler`, and no callable gate ever reaches that. The
|
||||||
|
* limitation is closed here rather than carried.
|
||||||
|
*
|
||||||
|
* Only LOCALS are affected either way: the prefix comes from
|
||||||
|
* `enclosingCallablePrefix`, which returns `undefined` when nothing encloses
|
||||||
|
* the declaration, so top-level and class-member ids are untouched — that is
|
||||||
|
* what keeps this off the symbols other files and stored references address.
|
||||||
|
* A class field stays unqualified even inside a function, because the prefix
|
||||||
|
* walk boundaries on class-likes.
|
||||||
|
*
|
||||||
|
* ONE definition, deliberately: the id-building phase and the resolution phase
|
||||||
|
* must agree on this set or the caller attaches to a node that does not exist
|
||||||
|
* and the edge is silently dropped — the failure mode #2714 fixed, invisible
|
||||||
|
* from outside because "zero dangling edges" is what it looks like.
|
||||||
|
*/
|
||||||
|
export function isPositionQualifiedLocalLabel(label: NodeLabel | undefined): boolean {
|
||||||
|
return (
|
||||||
|
isOverloadableCallable(label) ||
|
||||||
|
label === 'Variable' ||
|
||||||
|
label === 'Const' ||
|
||||||
|
label === 'Property' ||
|
||||||
|
label === 'Static'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -100,6 +100,7 @@ import {
|
||||||
LOCAL_SCOPE_BODY_NODE_TYPES,
|
LOCAL_SCOPE_BODY_NODE_TYPES,
|
||||||
type SyntaxNode,
|
type SyntaxNode,
|
||||||
} from '../utils/ast-helpers.js';
|
} from '../utils/ast-helpers.js';
|
||||||
|
import { isPositionQualifiedLocalLabel } from '../utils/callable-labels.js';
|
||||||
import { extractCallArgTypes, type MixedChainStep } from '../utils/call-analysis.js';
|
import { extractCallArgTypes, type MixedChainStep } from '../utils/call-analysis.js';
|
||||||
import { buildTypeEnv } from '../type-env.js';
|
import { buildTypeEnv } from '../type-env.js';
|
||||||
import type { ConstructorBinding } from '../type-env.js';
|
import type { ConstructorBinding } from '../type-env.js';
|
||||||
|
|
@ -2297,13 +2298,16 @@ const processFileGroup = (
|
||||||
// #2699: a callable nested inside another callable is qualified by the
|
// #2699: a callable nested inside another callable is qualified by the
|
||||||
// enclosing callable, so a function-local closure stops colliding with a
|
// enclosing callable, so a function-local closure stops colliding with a
|
||||||
// same-named file-level function. Restricted to CALLABLE labels: the
|
// same-named file-level function. Restricted to CALLABLE labels: the
|
||||||
// collision that produced wrong CALLS edges is between callables, and
|
// Applies to VALUES as well as callables since #2699 closed A1: a
|
||||||
// widening it to every function-local Variable/Property would churn ids
|
// top-level `const handler` and a function-local `const handler`
|
||||||
// for symbols the local-symbol pruner mostly deletes anyway.
|
// otherwise collapse onto one `Const:v.ts:handler`, which was the
|
||||||
|
// issue's original complaint and is unreachable from a callable-only
|
||||||
|
// gate. `isPositionQualifiedLocalLabel` is the single definition of that
|
||||||
|
// set, shared with resolution in `ids.ts` — the two phases disagreeing
|
||||||
|
// silently drops edges rather than failing (#2714).
|
||||||
// Same helper as the caller-attribution phase — see `enclosingCallablePrefix`.
|
// Same helper as the caller-attribution phase — see `enclosingCallablePrefix`.
|
||||||
const nestedCallablePrefix =
|
const nestedCallablePrefix =
|
||||||
(nodeLabel === 'Function' || nodeLabel === 'Method' || nodeLabel === 'Constructor') &&
|
isPositionQualifiedLocalLabel(nodeLabel) && definitionNode
|
||||||
definitionNode
|
|
||||||
? enclosingCallablePrefix(definitionNode, file.path, provider)
|
? enclosingCallablePrefix(definitionNode, file.path, provider)
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -55,6 +55,18 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j
|
||||||
// the main thread (the #1983 OOM). Because the two stores share this version,
|
// the main thread (the #1983 OOM). Because the two stores share this version,
|
||||||
// any future change to the `ParsedFile` serialization shape MUST bump
|
// any future change to the `ParsedFile` serialization shape MUST bump
|
||||||
// SCHEMA_BUMP so both invalidate in lockstep.
|
// SCHEMA_BUMP so both invalidate in lockstep.
|
||||||
|
// v29: closure-binding declaration rules for PHP/Rust/Kotlin/Ruby/Dart, a Rust
|
||||||
|
// graph node for `let f = || …`, a Dart closure scope, and function-local VALUES
|
||||||
|
// (Variable/Const/Property/Static) qualified by their enclosing callable plus
|
||||||
|
// position (#2699 parts A1 + B). All parse-time, so a warm cache would replay
|
||||||
|
// the old captures and the pre-qualification ids verbatim.
|
||||||
|
//
|
||||||
|
// This is 29 and not 28 because of the exact collision the v21 note below warns
|
||||||
|
// about: this branch cut at 27 and bumped to 28, while #2415 bumped 27 -> 28 and
|
||||||
|
// merged FIRST. Re-checking against origin/main at merge time — not at branch
|
||||||
|
// time — is what caught it; leaving it at 28 would have shipped this change with
|
||||||
|
// NO parse-cache invalidation, so every warm cache keeps serving the pre-fix
|
||||||
|
// captures and ids.
|
||||||
// v28: Java/Kotlin capture side-channels persist Spring condition facts and
|
// v28: Java/Kotlin capture side-channels persist Spring condition facts and
|
||||||
// annotation-source line numbers (#2415).
|
// annotation-source line numbers (#2415).
|
||||||
// v26: the enclosing-callable walk stops at class bodies and anonymous-class
|
// v26: the enclosing-callable walk stops at class bodies and anonymous-class
|
||||||
|
|
@ -103,7 +115,7 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j
|
||||||
// JLS 13.1 immediate-host chains (#2555).
|
// JLS 13.1 immediate-host chains (#2555).
|
||||||
// v18: Worker$N anonymous bodies. v17: callable-value-flow operand identity.
|
// v18: Worker$N anonymous bodies. v17: callable-value-flow operand identity.
|
||||||
// v16: direct callee identity.
|
// v16: direct callee identity.
|
||||||
const SCHEMA_BUMP = 28;
|
const SCHEMA_BUMP = 29;
|
||||||
const GITNEXUS_PKG_VERSION = (() => {
|
const GITNEXUS_PKG_VERSION = (() => {
|
||||||
try {
|
try {
|
||||||
// package.json sits at gitnexus/package.json — two levels up from
|
// package.json sits at gitnexus/package.json — two levels up from
|
||||||
|
|
|
||||||
|
|
@ -525,8 +525,16 @@ export interface RepoMeta {
|
||||||
* reads it also covered are kept. A v19 index holds those false CALLS/ACCESSES on
|
* reads it also covered are kept. A v19 index holds those false CALLS/ACCESSES on
|
||||||
* every unchanged file and would keep serving them through the reuse gate; force a
|
* every unchanged file and would keep serving them through the reuse gate; force a
|
||||||
* full re-analyze instead.
|
* full re-analyze instead.
|
||||||
|
* v21: a closure bound to a name is a call SOURCE in every language, not only a
|
||||||
|
* TARGET (#2699 part B). PHP/Rust/Kotlin/Ruby/Dart closure bindings gained the
|
||||||
|
* declaration rule, Rust gained the graph NODE it never emitted, and Dart locals
|
||||||
|
* gained the enclosing-callable + position identity that made two same-named
|
||||||
|
* closures collapse onto one node — which had them asserting a CALLS edge
|
||||||
|
* present nowhere in the source. All of that changes emitted node ids AND edges
|
||||||
|
* on files that did not themselves change, so a v20 index topped up
|
||||||
|
* incrementally keeps serving the old attribution; force a full re-analyze.
|
||||||
*/
|
*/
|
||||||
export const INCREMENTAL_SCHEMA_VERSION = 20;
|
export const INCREMENTAL_SCHEMA_VERSION = 21;
|
||||||
|
|
||||||
export interface IndexedRepo {
|
export interface IndexedRepo {
|
||||||
repoPath: string;
|
repoPath: string;
|
||||||
|
|
|
||||||
|
|
@ -307,24 +307,32 @@ const valueNodeIdsFor = async (
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
describeIfWorkerBuilt('KNOWN LIMIT — the identity fix is callable-restricted (#2699)', () => {
|
describeIfWorkerBuilt('function-local VALUES carry their own identity (#2699 A1)', () => {
|
||||||
it('a function-local VALUE still collapses onto the file-level node', async () => {
|
it('a function-local VALUE does not collapse onto the file-level node', async () => {
|
||||||
// Pinned as a limitation, not asserted as correct. #2695 gave function-local
|
// FLIPPED, per this test's own former instruction. It previously pinned the
|
||||||
// CALLABLES a position-bearing id; VALUES were deliberately excluded —
|
// collapse as a KNOWN LIMIT: #2695 gave function-local CALLABLES a
|
||||||
// widening it would re-key ~14,700 build-time nodes to change ~800 persisted
|
// position-bearing id and deliberately excluded VALUES, so a top-level
|
||||||
// ones, because the pruner deletes most of them (see the decision recorded
|
// `const handler` and a function-local `const handler` shared ONE node.
|
||||||
// in `workers/parse-worker.ts`). The guard that fails closed on a position
|
// That was the residual half of #2699's ORIGINAL complaint — the issue is
|
||||||
// miss is likewise gated on `isOverloadableCallable`
|
// about values first, and no callable-only gate could ever reach it.
|
||||||
// (Function | Method | Constructor), so a value never reaches it.
|
|
||||||
//
|
//
|
||||||
// Consequence, measured here: a top-level `const handler` and a
|
// Widened here via `isPositionQualifiedLocalLabel`, the single definition
|
||||||
// function-local `const handler` share ONE node. That is the residual half
|
// shared by all THREE phases that must agree: id-building
|
||||||
// of #2699's original complaint, and it is why the issue is not fully
|
// (`parse-worker.ts`), resolution (`ids.ts` position key) and registration
|
||||||
// closed by the identity work alone.
|
// (`node-lookup.ts`). Two of them disagreeing does not fail loudly — the
|
||||||
|
// caller attaches to a node that does not exist and the edge is silently
|
||||||
|
// dropped, which is the #2714 failure mode.
|
||||||
//
|
//
|
||||||
// If this ever returns two ids, the identity model was widened to values —
|
// The churn this was deferred for is real and was accepted deliberately:
|
||||||
// which is a deliberate, schema-bumping change, so this test should be
|
// it re-keys ~14,700 build-time nodes to change ~800 persisted ones,
|
||||||
// updated as part of it rather than deleted.
|
// because `pruneLocalSymbols` deletes most locals. Hence the paired
|
||||||
|
// INCREMENTAL_SCHEMA_VERSION / parse-cache SCHEMA_BUMP bumps — without them
|
||||||
|
// a warm cache or an incremental top-up replays the old un-suffixed ids.
|
||||||
|
//
|
||||||
|
// Only LOCALS move. The prefix comes from `enclosingCallablePrefix`, which
|
||||||
|
// returns undefined when nothing encloses the declaration, so the
|
||||||
|
// file-level `handler` below keeps its bare id — that is what keeps this
|
||||||
|
// off the symbols other files and stored references address.
|
||||||
const ids = await valueNodeIdsFor(
|
const ids = await valueNodeIdsFor(
|
||||||
'v.ts',
|
'v.ts',
|
||||||
[
|
[
|
||||||
|
|
@ -339,6 +347,8 @@ describeIfWorkerBuilt('KNOWN LIMIT — the identity fix is callable-restricted (
|
||||||
'handler',
|
'handler',
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(ids).toEqual(['Const:v.ts:handler']);
|
// Two distinct nodes: the file-level one keeps its bare id, the local
|
||||||
|
// carries its enclosing callable AND declaration position.
|
||||||
|
expect(ids).toEqual(['Const:v.ts:handler', 'Const:v.ts:run.handler@3:2']);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -64,8 +64,13 @@ describe('no call site re-inlines the rule', () => {
|
||||||
it('parse-worker.ts contains no inlined `<prefix>.${localIdentity(...)}` template', () => {
|
it('parse-worker.ts contains no inlined `<prefix>.${localIdentity(...)}` template', () => {
|
||||||
// The structural half. The unit assertions above would still pass if a
|
// The structural half. The unit assertions above would still pass if a
|
||||||
// fourth phase appeared and spelled the rule out by hand — which is
|
// fourth phase appeared and spelled the rule out by hand — which is
|
||||||
// exactly how the divergence #2714 fixed came to exist. This fails if any
|
// exactly how the divergence #2714 fixed came to exist.
|
||||||
// site reconstructs the id instead of calling the shared function.
|
//
|
||||||
|
// Scope, stated honestly: this matches ONE template spelling — the
|
||||||
|
// `${prefix}.${localIdentity(...)}` form the divergence actually took. A
|
||||||
|
// hand-rolled id built by string concatenation, or with the interpolation
|
||||||
|
// spelled differently, still slips past. It is a tripwire for the known
|
||||||
|
// shape, not a proof that no site reconstructs the id.
|
||||||
const source = readFileSync(
|
const source = readFileSync(
|
||||||
fileURLToPath(new URL('../../src/core/ingestion/workers/parse-worker.ts', import.meta.url)),
|
fileURLToPath(new URL('../../src/core/ingestion/workers/parse-worker.ts', import.meta.url)),
|
||||||
'utf8',
|
'utf8',
|
||||||
|
|
|
||||||
99
gitnexus/test/unit/receiver-twin-list-drift.test.ts
Normal file
99
gitnexus/test/unit/receiver-twin-list-drift.test.ts
Normal file
|
|
@ -0,0 +1,99 @@
|
||||||
|
/**
|
||||||
|
* The drift guard for the implicit-receiver twin lists (#2699 follow-up).
|
||||||
|
*
|
||||||
|
* TWO lists spell "this is an implicit receiver", in two packages:
|
||||||
|
*
|
||||||
|
* - `IMPLICIT_RECEIVERS` — gitnexus-shared `lookup-core.ts`. Two consumers:
|
||||||
|
* the Step-1 lexical skip (a NAMED receiver must not resolve its member
|
||||||
|
* through the lexical chain) and `resolveReceiverOwner`.
|
||||||
|
* - `THIS_RECEIVERS` — gitnexus `type-env.ts`. Decides whether a receiver
|
||||||
|
* rewrites to the enclosing type.
|
||||||
|
*
|
||||||
|
* They are the SIXTH twin-list instance found in this family of work, and the
|
||||||
|
* previous five each shipped a bug when one side moved. `$this` was added to
|
||||||
|
* the shared list in #2714 precisely because it was already in the other one;
|
||||||
|
* nothing but this test stops the next divergence.
|
||||||
|
*
|
||||||
|
* `Me` is the one deliberate asymmetry: `THIS_RECEIVERS` carries it (Visual
|
||||||
|
* Basic spelling) and the shared list does not, because no entry in
|
||||||
|
* `SupportedLanguages` uses it — mirroring it there could only ever exempt a
|
||||||
|
* variable that happens to be named `Me`. That exemption is asserted
|
||||||
|
* explicitly rather than tolerated, so RE-adding `Me` to the shared list, or
|
||||||
|
* dropping it from the local one, both fail loudly.
|
||||||
|
*
|
||||||
|
* Structural (source-parsed) rather than value-imported: both constants are
|
||||||
|
* module-private, and exporting them purely to be testable would widen two
|
||||||
|
* public surfaces to satisfy a test. Same idiom as
|
||||||
|
* `detect-changes-local-id-stability.test.ts`.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
import { readFileSync } from 'fs';
|
||||||
|
import path from 'path';
|
||||||
|
import { fileURLToPath } from 'url';
|
||||||
|
|
||||||
|
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||||
|
|
||||||
|
/**
|
||||||
|
* String literals inside the first `[...]` following `marker` that actually
|
||||||
|
* CONTAINS a string literal.
|
||||||
|
*
|
||||||
|
* "First `[`" is not good enough: `IMPLICIT_RECEIVERS` is declared
|
||||||
|
* `: readonly string[] = Object.freeze([...])`, so the first bracket belongs to
|
||||||
|
* the TYPE annotation and yields an empty list — which would make every
|
||||||
|
* assertion below vacuously pass. That is exactly what the non-empty check in
|
||||||
|
* the first test exists to catch, and it did.
|
||||||
|
*/
|
||||||
|
const literalsAfter = (source: string, marker: string): string[] => {
|
||||||
|
const at = source.indexOf(marker);
|
||||||
|
expect(at, `${marker} not found — update this test`).toBeGreaterThan(-1);
|
||||||
|
for (let open = source.indexOf('[', at); open !== -1; open = source.indexOf('[', open + 1)) {
|
||||||
|
const close = source.indexOf(']', open);
|
||||||
|
if (close === -1) break;
|
||||||
|
const names = [...source.slice(open + 1, close).matchAll(/'([^']*)'|"([^"]*)"/g)]
|
||||||
|
.map((m) => m[1] ?? m[2] ?? '')
|
||||||
|
.filter((s) => s.length > 0);
|
||||||
|
if (names.length > 0) return names.sort();
|
||||||
|
}
|
||||||
|
return [];
|
||||||
|
};
|
||||||
|
|
||||||
|
const sharedList = (): string[] =>
|
||||||
|
literalsAfter(
|
||||||
|
readFileSync(
|
||||||
|
path.join(
|
||||||
|
__dirname,
|
||||||
|
'../../../gitnexus-shared/src/scope-resolution/registries/lookup-core.ts',
|
||||||
|
),
|
||||||
|
'utf-8',
|
||||||
|
),
|
||||||
|
'const IMPLICIT_RECEIVERS',
|
||||||
|
);
|
||||||
|
|
||||||
|
const typeEnvList = (): string[] =>
|
||||||
|
literalsAfter(
|
||||||
|
readFileSync(path.join(__dirname, '../../src/core/ingestion/type-env.ts'), 'utf-8'),
|
||||||
|
'const THIS_RECEIVERS',
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('#2699 — implicit-receiver twin lists do not drift', () => {
|
||||||
|
it('both lists are non-empty and were actually parsed', () => {
|
||||||
|
// Guards the guard: a regex that silently matched nothing would make every
|
||||||
|
// assertion below vacuously true.
|
||||||
|
expect(sharedList().length).toBeGreaterThan(0);
|
||||||
|
expect(typeEnvList().length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('the shared list is exactly the type-env list minus the deliberate `Me`', () => {
|
||||||
|
expect(sharedList()).toEqual(typeEnvList().filter((name) => name !== 'Me'));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('`Me` stays OUT of the shared list', () => {
|
||||||
|
// Stated separately so the intent survives even if the set comparison above
|
||||||
|
// is ever relaxed: this asymmetry is a decision, not an oversight.
|
||||||
|
expect(sharedList()).not.toContain('Me');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('`Me` stays IN the type-env list', () => {
|
||||||
|
expect(typeEnvList()).toContain('Me');
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Add table
Reference in a new issue