Implement tracked correctness fixes#134
Open
lzehrung wants to merge 4 commits into
Open
Conversation
Contributor
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Resolved IssuesThe following previous observations were addressed in this incremental diff:
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed in Increment (14 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 8d99c99)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8d99c99)Status: No Issues Found | Recommendation: Merge Incremental ReviewPrevious review at Carried-Forward Observations2 previous observations on unchanged files remain active (see existing inline comments). Files Reviewed in Increment (16 files)
Previous review (commit a5b4fbf)Status: No Issues Found | Recommendation: Merge Incremental ReviewPrevious review at Carried-Forward Observations2 previous observations on unchanged files remain active (see existing inline comments). Files Reviewed in Increment (16 files)
Previous review (commit a9fcb0e)Status: No Issues Found | Recommendation: Merge Files Reviewed (13 files)
Reviewed by kimi-k2.6-20260420 · Input: 535.1K · Output: 46.2K · Cached: 1.4M |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements a set of “tracked correctness” fixes across the JS/TS indexer and impact suggestion pipeline, with accompanying tests and documentation updates to reflect the intended installation/runtime behavior.
Changes:
- Improve JS/TS semantic navigation by indexing TypeScript enums, anonymous default exports, and shorthand-property bindings for goto/refs.
- Enhance breaking-change suggestions by detecting multiline exported function arity changes from diffs.
- Clarify installation/runtime docs around the JS fallback shim and mark tracked items complete.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/references.test.ts | Adds coverage for finding references to exported TypeScript enums. |
| tests/impact-suggestions.test.ts | Adds a regression test for multiline exported function arity change detection. |
| tests/goto.test.ts | Adds goto-definition coverage for TS enums, anonymous default exports, and shorthand properties. |
| src/languages/definitions/typescript.ts | Extends TS queries/node-types to index enums, anonymous default exports, and shorthand-property identifier patterns. |
| src/languages/definitions/javascript.ts | Extends JS queries/node-types to capture anonymous default exports and shorthand-property identifier patterns. |
| src/indexer/scope.ts | Improves scope binding collection for destructuring/shorthand patterns and treats enums as type bindings. |
| src/indexer/navigation.ts | Treats shorthand-property identifier nodes as navigable identifiers for goto-definition. |
| src/indexer/locals-and-exports.ts | Improves default-export symbol extraction (including anonymous defaults) and TSX default-export fallback behavior. |
| src/impact/report-suggestions.ts | Reworks exported-signature parsing to detect multiline signatures within diff hunks. |
| README.md | Clarifies that @lzehrung/codegraph-js-fallback is a compatibility shim (not a parser bundle). |
| docs/plans/2026-06-06-performance-and-cache-opportunities.md | Checks off the tracked correctness items addressed by this PR. |
| docs/installation.md | Clarifies reduced-mode behavior and that the JS fallback package does not restore parsing. |
| codegraph-skill/codegraph/SKILL.md | Updates agent guidance to avoid recommending the shim as a grammar fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+179
to
+183
| node.type === "interface_declaration" || | ||
| node.type === "type_alias_declaration" || | ||
| node.type === "type_spec" || | ||
| node.type === "trait_item" | ||
| node.type === "trait_item" || | ||
| node.type === "enum_declaration" |
Comment on lines
+11
to
+14
| public enum Mode { | ||
| FAST, | ||
| SLOW | ||
| } |
| - [x] Anonymous JS/TS default exports should resolve through default imports. | ||
| - [x] JS/TS shorthand binding nodes should navigate to the actual binding/import. | ||
| - [x] Breaking-change suggestions should handle multiline exported signatures. | ||
| - [x] Publishing/install docs should clarify that JS fallback is only available when `@lzehrung/codegraph-js-fallback` is installed. |
Comment on lines
+129
to
+152
| const addUnsupportedRequirePatternDecls = (pattern: SyntaxNodeLike): void => { | ||
| if (idSet.has(pattern.type)) return; | ||
| if (pattern.type !== "object_pattern") { | ||
| addPatternDecls(pattern, "local"); | ||
| return; | ||
| } | ||
| for (const child of pattern.namedChildren) { | ||
| if (child.type === "shorthand_property_identifier" || child.type === "shorthand_property_identifier_pattern") { | ||
| continue; | ||
| } | ||
| if (child.type === "pair_pattern") { | ||
| const key = child.childForFieldName("key"); | ||
| const value = child.childForFieldName("value"); | ||
| if (key && value && key.type === "property_identifier" && value.type === "identifier") { | ||
| continue; | ||
| } | ||
| if (value) { | ||
| addPatternDecls(value, "local"); | ||
| } | ||
| continue; | ||
| } | ||
| addPatternDecls(child, "local"); | ||
| } | ||
| }; |
Comment on lines
778
to
+783
| const defIdent = source.match(/\bexport\s+default\s+([A-Za-z_$][\w$]*)\b/); | ||
| const name = defFn?.[1] ?? defCls?.[1] ?? defIdent?.[1]; | ||
| const identName = | ||
| defIdent && defIdent[1] !== "function" && defIdent[1] !== "class" && defIdent[1] !== "abstract" | ||
| ? defIdent[1] | ||
| : undefined; | ||
| const name = defFn?.[1] ?? defCls?.[1] ?? identName; |
Comment on lines
+610
to
+612
| const functionRe = | ||
| /^\s*export\s+(?:default\s+)?(?:async\s+)?function(?:\s+([A-Za-z_$][\w$]*))?(?:\s*<[^>]+>)?\s*\(([\s\S]*?)\)/gm; | ||
| for (let match; (match = functionRe.exec(text)); ) { |
Comment on lines
209
to
210
| let pushed = false; | ||
| if (support.createsFunctionScope(node)) { |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ) { | ||
| const name = node.childForFieldName("name"); | ||
| if (name) { | ||
| if (node.type === "enumerator" && support.id === "c") addDecl(name, "local"); |
Contributor
There was a problem hiding this comment.
WARNING: C++ enumerators are double-indexed in scope results
C++ (support.id === "cpp") is a customDeclLanguage; its isDeclarationName already treats enumerator names as declarations, so the custom declaration path (lines 289-291) adds C++ enum constants to the scope map. Without including "cpp" here, the else branch also pushes them to extraBindings, causing duplicate entries in the final bindings map.
Suggested change
| if (node.type === "enumerator" && support.id === "c") addDecl(name, "local"); | |
| if (node.type === "enumerator" && (support.id === "c" || support.id === "cpp")) addDecl(name, "local"); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification