From b9a4d7edff70c95e48589ca40cf5150ab9be2103 Mon Sep 17 00:00:00 2001 From: Borozdenets Ilya Date: Fri, 28 Aug 2026 13:40:45 +0300 Subject: [PATCH] fix(group): admit backtick-quoted constants, and overlay a same-file val that imports nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gate defects, both reported by the review bot and both reproduced before being fixed. `isKotlinConstantFile` matched only `\w+` for a declaration's name, so a file whose constants are backtick-quoted — `const val ` + "`ORDERS`" + ` = "/orders"` — failed both arms and was never parsed into the repo constant map. The resolver supports backtick identifiers everywhere else: `unquoteKotlinIdentifier` strips the quoting at every point a name becomes a key or a lookup. So the gate was NARROWER than the extractor, which is the one direction its arms exist to exclude, and a cross-file reference to such a constant floored to skip. Measured: the route emitted nothing, and emits `GET /orders` now. The on-demand overlay in `scan` admitted the file's extraction only when it had imports. A file declaring a top-level non-`const` `val` is already excluded from the pre-pass map (no `const`, no `object`), so this branch is its only chance, and an import-only test discarded exactly the constants the route needed. The guard now matches the admission test the pre-pass itself applies. The bot stated this second one more broadly than it holds. Measured, any import at all masks it — a realistic Spring controller always has one — so the failure needs all three of: a top-level non-`const` `val`, no `object` in the file, and no imports. Narrow, but real, and the fix costs one predicate. Verified with the differential probe: both cases go from no detection to the correct route, and all 41 existing fixtures are byte-identical before and after. Co-Authored-By: Claude Opus 5 (1M context) --- .../group/extractors/http-patterns/kotlin.ts | 10 +++- .../route-extractors/kotlin-const-resolver.ts | 14 ++++- .../group/kotlin-const-route-fold.test.ts | 51 +++++++++++++++++++ .../unit/kotlin-route-const-resolver.test.ts | 30 +++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index 37adece51..fbce99e19 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -1374,7 +1374,15 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { if (kotlinCtx.constants.has(fileKey)) return foldConstants; try { const mc = extractKotlinModuleConstants(tree); - if (mc.imports.size > 0) { + // Same admission test the pre-pass applies above. It used to be + // `imports.size > 0` alone, which dropped a file declaring a top-level + // non-`const` `val` and importing nothing: the gate excludes it from + // the pre-pass map (no `const`, no `object`), so this branch is its + // only chance, and an import-only test threw it away. Any import at + // all masked the bug, which is why a realistic controller never hit + // it — the failure needs a file with a top-level `val`, no `object` + // and no imports. + if (mc.literals.size > 0 || mc.exprs.size > 0 || mc.imports.size > 0) { const merged = new Map(kotlinCtx.constants); merged.set(fileKey, mc); foldConstants = merged; diff --git a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts index dc6f93306..364ef8f89 100644 --- a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts @@ -279,10 +279,20 @@ const MAX_FOLD_DEPTH = 32; * contains any `val … =`, function locals included — very nearly the whole * repository, in a pass whose entire purpose is to avoid parsing it. That trade * has not been measured, so the gap is recorded here rather than papered over. + * + * Both name arms accept a BACKTICK-QUOTED identifier as well as a bare one, + * because the extractor does: `unquoteKotlinIdentifier` strips the quoting + * everywhere a name becomes a key, so `const val \`ORDERS\` = "/orders"` is a + * constant this module resolves. A gate that matched only `\w+` rejected the + * file outright and the reference floored to skip — a gate narrower than the + * extractor, which is the one direction the arms above are meant to exclude. */ -const CONST_VAL_RE = /\bconst\s+val\s+\w+\s*(?::[^=\n{}()]{0,60})?=/; +const KOTLIN_NAME = String.raw`(?:\w+|\`[^\`\n]+\`)`; +const CONST_VAL_RE = new RegExp( + String.raw`\bconst\s+val\s+${KOTLIN_NAME}\s*(?::[^=\n{}()]{0,60})?=`, +); const OBJECT_DECL_RE = /\bobject\b/; -const VAL_BINDING_RE = /\bval\s+\w+\s*(?::[^=\n{}()]{0,60})?=/; +const VAL_BINDING_RE = new RegExp(String.raw`\bval\s+${KOTLIN_NAME}\s*(?::[^=\n{}()]{0,60})?=`); export function isKotlinConstantFile(source: string): boolean { if (CONST_VAL_RE.test(source)) return true; diff --git a/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts b/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts index b50652625..0365c9ceb 100644 --- a/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts +++ b/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts @@ -933,6 +933,57 @@ object ApiPaths { ).toEqual(['GET /api/v1/orders']); }); + it('folds a same-file top-level `val` even when the file imports nothing', () => { + // Three conditions have to line up for this to break, which is why a + // realistic controller never hit it: the constant is a top-level non-`const` + // `val` (so `isKotlinConstantFile` rejects the file and the pre-pass never + // indexes it), the file declares no `object` (the gate's other arm), and it + // imports nothing. The on-demand overlay in `scan` is the file's only + // remaining chance, and it used to admit the extraction only when the file + // had imports — throwing away the very constants the route needs. + expect( + providers({ + [CONTROLLER]: `package com.example.app.web + +val PATH = "/orders" + +@RestController +@RequestMapping("/api/v1") +class OrderController { + @GetMapping(PATH) + fun list() {} +} +`, + }), + ).toEqual(['GET /api/v1/orders']); + }); + + it('folds a backtick-quoted constant declared in its own file', () => { + // The gate decides whether a file is parsed into the repo map at all, so a + // gate that rejects backticks silently drops a constant the resolver can + // fold — a cross-file reference to it then floors to skip. + expect( + providers({ + [CONSTS]: `package com.example.app.api + +object ApiPaths { + const val \`ORDERS\` = "/api/v1/orders" +} +`, + [CONTROLLER]: `package com.example.app.web + +import com.example.app.api.ApiPaths + +@RestController +class OrderController { + @GetMapping(ApiPaths.ORDERS) + fun list() {} +} +`, + }), + ).toEqual(['GET /api/v1/orders']); + }); + it('leaves literal routes unchanged and emits each exactly once', () => { expect( providers({ diff --git a/gitnexus/test/unit/kotlin-route-const-resolver.test.ts b/gitnexus/test/unit/kotlin-route-const-resolver.test.ts index 8bb170eec..e7e6deeaf 100644 --- a/gitnexus/test/unit/kotlin-route-const-resolver.test.ts +++ b/gitnexus/test/unit/kotlin-route-const-resolver.test.ts @@ -1182,6 +1182,36 @@ object \`ApiPaths\` { class OrderService { fun list(): List = emptyList() } +`), + ).toBe(false); + }); + + it('admits a backtick-quoted name, because the extractor resolves one', () => { + // A gate NARROWER than the extractor costs a fact: the file is never + // parsed into the repo map, so a cross-file reference to the constant + // floors to skip. `unquoteKotlinIdentifier` strips the quoting everywhere + // a name becomes a key, so these declarations are ones this module folds. + expect(isKotlinConstantFile('const val `ORDERS` = "/api/v1/orders"')).toBe(true); + expect(isKotlinConstantFile('object O { val `ORDERS`: String = "/api/v1/orders" }')).toBe( + true, + ); + expect( + isKotlinConstantFile('class C { companion object { const val `O` = "/orders" } }'), + ).toBe(true); + }); + + it('does not let the backtick arm widen into a file with no carrier', () => { + // The control for the arm above: accepting backticks must not turn the + // gate into "any file mentioning val", which is the whole repository. + expect( + isKotlinConstantFile(`package com.example.app.web + +class OrderService { + fun list(): List { + val \`local name\` = "not a constant" + return listOf(\`local name\`) + } +} `), ).toBe(false); });