-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(coding-agent/tools): added ast_grep empty-result hints and enriched AST descriptions #2665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
metaphorics
wants to merge
1
commit into
can1357:main
Choose a base branch
from
metaphorics:ultra/ast
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import { detectLanguageSpecificMistake, getPatternHint, inferAstLanguage } from "./ast-pattern-hints"; | ||
|
|
||
| /** | ||
| * The empty-match hints turn the two most common `ast_grep` mistakes — regex | ||
| * leaking into an AST pattern, and a structurally incomplete declaration — into | ||
| * a one-line nudge. These lock the detection contract: every regex construct is | ||
| * caught and routed to `search`, valid patterns stay silent (no false nudge), | ||
| * regex detection outranks the language check, and language inference only | ||
| * commits when the extensions agree. | ||
|
Comment on lines
+5
to
+10
|
||
| */ | ||
| describe("getPatternHint regex misuse", () => { | ||
| it("flags alternation and routes to search", () => { | ||
| const hint = getPatternHint("foo|bar", undefined); | ||
| expect(hint).toContain("alternation"); | ||
| expect(hint).toContain("search"); | ||
| }); | ||
|
|
||
| it("flags wildcards, escapes, and character classes", () => { | ||
| expect(getPatternHint("a.*b", undefined)).toContain("wildcard"); | ||
| expect(getPatternHint("foo\\w", undefined)).toContain("regex escapes"); | ||
| expect(getPatternHint("name[a-z]", undefined)).toContain("character classes"); | ||
| }); | ||
|
|
||
| it("stays silent for a valid AST pattern", () => { | ||
| expect(getPatternHint("console.log($$$)", undefined)).toBeUndefined(); | ||
| expect(getPatternHint("const $X = $Y", "typescript")).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("prefers the regex hint over the language hint", () => { | ||
| // `foo|bar` is alternation regardless of language context. | ||
| expect(getPatternHint("foo|bar", "python")).toContain("alternation"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("getPatternHint language-specific shape", () => { | ||
| it("flags a Python declaration with a trailing colon and shows the fix", () => { | ||
| const hint = getPatternHint("def $F($$$):", "python"); | ||
| expect(hint).toContain("trailing colon"); | ||
| expect(hint).toContain('"def $F($$$)"'); | ||
| }); | ||
|
|
||
| it("flags a bare function pattern lacking params and body", () => { | ||
| expect(detectLanguageSpecificMistake("function $NAME", "typescript")).toContain("params and a body"); | ||
| expect(detectLanguageSpecificMistake("func $NAME", "go")).toContain("params and a body"); | ||
| expect(detectLanguageSpecificMistake("fn $NAME", "rust")).toContain("params and a body"); | ||
| }); | ||
|
|
||
| it("stays silent without a known language or for a complete pattern", () => { | ||
| expect(detectLanguageSpecificMistake("def $F($$$):", undefined)).toBeUndefined(); | ||
| expect(detectLanguageSpecificMistake("def $F($$$)", "python")).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("inferAstLanguage", () => { | ||
| it("resolves a single extension from globs and files", () => { | ||
| expect(inferAstLanguage(["src/**/*.py"])).toBe("python"); | ||
| expect(inferAstLanguage(["src/worker.ts"])).toBe("typescript"); | ||
| expect(inferAstLanguage(["cmd/main.go"])).toBe("go"); | ||
| }); | ||
|
|
||
| it("returns undefined when extensions disagree or are absent", () => { | ||
| expect(inferAstLanguage(["src/a.ts", "src/b.tsx"])).toBeUndefined(); | ||
| expect(inferAstLanguage(["src/"])).toBeUndefined(); | ||
| expect(inferAstLanguage([])).toBeUndefined(); | ||
| }); | ||
| }); | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /** | ||
| * Advisory hints for `ast_grep` patterns that returned zero matches. | ||
| * | ||
| * The single most common `ast_grep` failure is treating the pattern as regex | ||
| * or text — `\w`, `[a-z]`, `.*`, `foo|bar` parse as literal AST and never match. | ||
| * The second is a structurally incomplete declaration (a Python pattern with a | ||
| * trailing `:`, a bare `function $NAME` with no params/body). These detectors | ||
| * turn an empty result into a one-line nudge toward the right tool or shape. | ||
| * | ||
| * Every detector is best-effort and side-effect-free: it returns `undefined` | ||
| * when no rule matches, so the caller appends nothing rather than guessing. | ||
| */ | ||
|
|
||
| /** Language tokens the language-specific detector understands. */ | ||
| type AstHintLanguage = "python" | "javascript" | "typescript" | "tsx" | "go" | "rust"; | ||
|
|
||
| const EXTENSION_LANGUAGES: Record<string, AstHintLanguage> = { | ||
| py: "python", | ||
| ts: "typescript", | ||
| tsx: "tsx", | ||
| js: "javascript", | ||
| jsx: "javascript", | ||
| mjs: "javascript", | ||
| cjs: "javascript", | ||
| go: "go", | ||
| rs: "rust", | ||
| }; | ||
|
|
||
| /** | ||
| * Best-effort language inference from the search paths/globs. Returns a language | ||
| * only when every path with a recognized extension agrees on it; mixed or | ||
| * unrecognized inputs yield `undefined` (the language-specific hints then stay | ||
| * silent and only the regex-misuse hints can fire). | ||
| */ | ||
| export function inferAstLanguage(paths: readonly string[]): AstHintLanguage | undefined { | ||
| let resolved: AstHintLanguage | undefined; | ||
| for (const candidate of paths) { | ||
| const ext = /\.([a-z0-9]+)$/i.exec(candidate)?.[1]?.toLowerCase(); | ||
| if (!ext) continue; | ||
| const lang = EXTENSION_LANGUAGES[ext]; | ||
| if (!lang) continue; | ||
| if (resolved && resolved !== lang) return undefined; | ||
| resolved = lang; | ||
| } | ||
| return resolved; | ||
| } | ||
|
|
||
| /** Detect regex/text constructs that do not work in ast-grep patterns. */ | ||
| export function detectRegexMisuse(pattern: string): string | undefined { | ||
| const src = pattern.trim(); | ||
|
|
||
| if (/\\[wWdDsSbB]/.test(src)) { | ||
| return 'Hint: "\\w", "\\d", "\\s", "\\b" are regex escapes. ast_grep matches AST nodes, not text — use $VAR for one identifier, $$$ for a node list, or the `search` tool for text.'; | ||
| } | ||
|
|
||
| if (/\[[a-zA-Z0-9]-[a-zA-Z0-9]\]/.test(src)) { | ||
| return 'Hint: "[a-z]" and similar character classes are regex, not AST. Use $VAR to match any identifier, or the `search` tool for text search.'; | ||
| } | ||
|
|
||
| if (!src.includes("$") && /\w\.[*+]/.test(src)) { | ||
| return 'Hint: ".*" and ".+" are regex wildcards. In ast_grep use $$$ for multiple AST nodes and $VAR for a single node; for text patterns use the `search` tool.'; | ||
| } | ||
|
Comment on lines
+60
to
+62
|
||
|
|
||
| if (/^[-\w.*]+\|[-\w.*|]+$/.test(src)) { | ||
| return 'Hint: "|" is regex alternation and does NOT work in ast_grep patterns. Either fire one ast_grep call per alternative, or use the `search` tool with a regex like "foo|bar".'; | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** Detect structurally incomplete declarations for a known language. */ | ||
| export function detectLanguageSpecificMistake(pattern: string, lang: AstHintLanguage | undefined): string | undefined { | ||
| if (!lang) return undefined; | ||
| const src = pattern.trim(); | ||
|
|
||
| if (lang === "python") { | ||
| if (src.startsWith("class ") && src.endsWith(":")) { | ||
| return `Hint: drop the trailing colon — ast_grep patterns are not full statements. Try: "${src.slice(0, -1)}"`; | ||
| } | ||
| if ((src.startsWith("def ") || src.startsWith("async def ")) && src.endsWith(":")) { | ||
| return `Hint: drop the trailing colon — ast_grep patterns are not full statements. Try: "${src.slice(0, -1)}"`; | ||
| } | ||
|
Comment on lines
+77
to
+82
|
||
| } | ||
|
|
||
| if (lang === "javascript" || lang === "typescript" || lang === "tsx") { | ||
| if (/^(export\s+)?(async\s+)?function\s+\$[A-Z_]+\s*$/i.test(src)) { | ||
| return 'Hint: function patterns need params and a body. Try "function $NAME($$$) { $$$ }".'; | ||
| } | ||
| } | ||
|
|
||
| if (lang === "go" && /^func\s+\$[A-Z_]+\s*$/i.test(src)) { | ||
| return 'Hint: Go function patterns need params and a body. Try "func $NAME($$$) { $$$ }".'; | ||
| } | ||
|
|
||
| if (lang === "rust" && /^fn\s+\$[A-Z_]+\s*$/i.test(src)) { | ||
| return 'Hint: Rust fn patterns need params and a body. Try "fn $NAME($$$) { $$$ }".'; | ||
| } | ||
|
Comment on lines
+85
to
+97
|
||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Return a one-line advisory hint for a zero-match `ast_grep` pattern, or | ||
| * `undefined` when no rule applies. Regex-misuse detection runs first (it needs | ||
| * no language); the language-specific shape check runs only when `lang` is known. | ||
| */ | ||
| export function getPatternHint(pattern: string, lang: AstHintLanguage | undefined): string | undefined { | ||
| return detectRegexMisuse(pattern) ?? detectLanguageSpecificMistake(pattern, lang); | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a file changes between the
ast_editpreview andresolveapply, this guidance says the apply is rejected as stale, but the apply handler actually rerunsrunAstEditOnce(... dryRun: false)before computingstalePreview, so a mismatched nonzero run can already have written partial or extra replacements before returning an error. The prompt should not tell the model that drift prevents mutation unless the apply path is changed to dry-run/check first or roll back.Useful? React with 👍 / 👎.