diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ed35a8..88a0811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,17 @@ All notable changes to [`codegraph-engine`](https://www.npmjs.com/package/codegr ## Unreleased ### Fixed +- **Every call to a removed function is reported**, not one per calling function (a function that called the removed one three times showed only one line). Found by the mutation benchmark. - **Diffs no longer inflate the blast radius.** An edit on a line that declares a local variable is a change to its function (it used to be attributed to the variable, which has no callers, so the function's blast radius came out empty), and a module-level edit such as an import line no longer counts every symbol in the file as changed. Symbols a diff adds in full are reported as new instead of as changes. The pull request comment now counts only callers outside the files the change edits, says plainly when a change only adds code, and gives a test command per ecosystem (`go test ./pkg`, `pytest`, `vitest`/`jest`). - **Python function-local imports** (`def f(): from x import y`) are scoped to that function instead of replacing the file's import of the same name, and shadow a same-named function outside it. Removes one `resolved` error each in Flask and Rich in the Jedi check. ### Added +- **Agent tools** (MCP): `codegraph_check_change` (give it the new text of the files you are about to write; reports calls left behind by a removed function, calls with the wrong number of arguments, removed public API, who is reached and which tests to run, without writing anything), `codegraph_verify_symbol` (does it exist, with what parameters; closest names if not) and `codegraph_repo_map`. The server's index now follows the files on disk. Measured in `docs/AGENT-EVAL.md`. +- **An optional model reading of the change** in the pull request comment (`anthropic-api-key` input, or `codegraph impact --ai` with `ANTHROPIC_API_KEY`): what the change does, whether it matches the description, and what to check. Off unless a key is set, never used for fork pull requests, and the diff, description and facts it sends are quoted as untrusted data. +- **Line comments on the pull request diff** for findings that have a line: a removed function that is still called (on its deleted definition), a signature that call sites no longer fit (on the new signature, naming the calls), public API removed. Replaced on each push, kept when answered. `inline-comments: all` adds a note on edits to widely used code. The action now analyses the pull request's own commit so line numbers match GitHub's diff. `codegraph impact --inline-out ` writes them as JSON. +- **A verdict at the top of the pull request comment**: 🔴 likely breaking, 🟠 may change public API, 🟡 changes shared code, 🟢 contained or additive. A changed signature is checked against how many arguments each call site passes, and the comment names the calls that no longer fit. Argument counts are recorded for TypeScript, JavaScript, Python and Go. +- **The pull request comment catches removed or renamed functions that are still called.** It indexes the base commit as well, and lists any function that existed before, is gone now, and has a surviving caller that still calls the old name. Works for TypeScript, JavaScript, Python and Go. `codegraph impact --no-compare` skips it. +- **The pull request comment groups callers by the edit that reaches them** (the riskiest edit first, top-level code marked as such), shows the test command once with "N of M test files", and says when a change touches most of the suite. - **Go support.** `.go` files are indexed (CLI, web app, MCP, pull request comments): packages resolved through `go.mod`, methods attached to their types, struct embedding and promoted methods, interfaces, typed variables through constructors, results, ranges and fields, and `//go:build` for linux/amd64. Checked against the Go type checker on Cobra, Gin, Logrus and gorilla/mux: 98.5% of the calls it resolves are found, and every one of 7,550 edges is correct (first run, before any fix: 96.8% and 7,336 of 7,339). Details and limits in `docs/ORACLE-EVAL.md`. - **Private repositories in the web app**: paste a GitHub personal access token on the home page. It stays in the browser tab, is sent only to `api.github.com`, and also raises GitHub's limit from 60 to 5,000 requests an hour. - **`codegraph mcp`**: an MCP server (stdio) so AI agents can ask what a change touches: impact of a symbol or of the current diff, callers and callees, symbol search, an overview of the riskiest code. diff --git a/README.md b/README.md index f64eff9..60dc547 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,8 @@ Targets accepted by `impact` / `slice`: `src/a.ts`, `src/a.ts#Foo.bar`, `src/a.t ## Use it with AI agents (MCP) +**Before an agent writes an edit**, `codegraph_check_change` takes the new file text and says what it would break (removed functions still called, calls with the wrong number of arguments, removed public API); `codegraph_verify_symbol` confirms a function exists with the signature the agent is about to use. On constructed breakage in this repo it caught 100% of removals (every call site found) and 88% of added required parameters with no false alarms; it cannot tell when a changed React component no longer fits its JSX uses. Details and limits: [docs/AGENT-EVAL.md](docs/AGENT-EVAL.md). + `codegraph mcp` is a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio. Agents such as Claude Code can ask what a change touches before they make it. Tools: `codegraph_impact`, `codegraph_diff_impact`, `codegraph_find`, `codegraph_callers_callees`, `codegraph_overview`, `codegraph_reindex`. @@ -120,6 +122,52 @@ jobs: # with: { min-confidence: probable, fail-above: 200 } ``` +It also compares with the base commit and opens with a **verdict** that says whether existing callers can fail. It is +evidence, not a guess at intent: it cannot tell a bug fix from a feature, only whether something that exists is now +broken. + +| Verdict | When | +|---|---| +| 🔴 likely breaking | a function was removed or renamed and a surviving caller still calls the old name; or a function now needs more arguments (or accepts fewer) and a call site passes a count that no longer fits (it names the file and line) | +| 🟠 may change public API | an exported function's argument counts changed, or an exported function was removed, but nothing in the repository is known to break | +| 🟡 changes shared code | it edits code that other files call, with no incompatible signature change | +| 🟢 contained / additive | it edits code nothing outside the edited files calls, or only adds new code (listing the new public symbols) | + +The findings that have a line are also left **on the diff** as review comments, because that is where a reviewer looks: a +removed function that is still called goes on its deleted definition (GitHub only allows comments on lines of the diff, +and the caller that was missed is in a file the pull request did not touch), and a changed signature goes on the new +signature, naming the calls that no longer fit. They are replaced on each push and left alone once someone has replied. +Only 🔴 and 🟠 findings get a line comment; `inline-comments: all` adds a note on edits to code many others depend on, +and `inline-comments: false` turns them off. The action analyses the pull request's own commit, not the merge commit, so +that its line numbers match GitHub's diff. + +### An optional reading by a model + +Everything above is a fact about the code's structure, which is why it can be trusted and also why it cannot say what a +change is *for*. With an Anthropic API key the comment gains a short **reading of the change**: what it does in plain +language, whether the diff matches what the author wrote in the description (it will notice a change the description does +not mention), and up to three things worth checking. The deterministic verdict and facts are handed to the model as ground +truth, and the verdict does not depend on it. + +```yaml + - uses: harsharajkumar-273/CodeGraph@main + with: + anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} +``` + +It is off unless that key is set. When on, **the diff of the source files, the pull request title and description, and the +comment's facts are sent to the Anthropic API** on every push, at a few thousand tokens in and at most 700 out. It never runs +for a pull request from a fork. The title, description and diff are written by whoever opens the pull request, so they are +quoted as untrusted data the model is told never to obey, and its answer is only ever shown as text (markup, links and +mentions are stripped). It can still be wrong or miss things, so read it as a first pass from a reviewer, not a check. Its +answers have not been evaluated here, because that needs a key: try it on a few of your own pull requests before relying on it. + +What it can't see: it compares argument *counts* only (not names, order or types), reports calls only (not imports or +type-only uses), and treats every top-level Python function as public. Checked on 132 real single commits from four +repositories (Requests, Gin, Hono and this one) it raised no 🔴 or 🟠, and on a rename and on a new required argument +injected into Requests, Hono and Gin it fired with the real call sites. It has not been run on many real pull requests +that contain a genuine breaking change. `codegraph impact --no-compare` skips the comparison. + `fail-above` makes the check fail when a change reaches more symbols than that. Pull requests from forks have a read-only token, so the result goes to the job summary instead of a comment. The same text is available locally with `codegraph impact --diff --range main...HEAD --format markdown`. diff --git a/action.yml b/action.yml index 49bfcb0..4340372 100644 --- a/action.yml +++ b/action.yml @@ -12,6 +12,15 @@ inputs: min-confidence: description: "Weakest link to count (resolved, probable or ambiguous)." default: probable + inline-comments: + description: "Line comments on the diff for findings that have a line (a removed function still called, a signature that call sites no longer fit, public API removed). Use 'all' to add a note on edits to code many others depend on, or 'false' for none." + default: 'true' + anthropic-api-key: + description: "Optional. An Anthropic API key (store it as a secret). When set, a model reads the diff and the description and adds a short reading of the change to the comment. Off when empty, and never used for a pull request from a fork. The source files of the diff and the description are sent to the Anthropic API." + default: '' + ai-model: + description: "The model that writes the reading (default claude-sonnet-5-5)." + default: '' fail-above: description: "Fail the check when more than this many symbols are affected. Leave empty to only comment." default: '' @@ -32,20 +41,41 @@ runs: working-directory: ${{ github.action_path }} env: BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} MIN: ${{ inputs.min-confidence }} WORKSPACE: ${{ github.workspace }} OUT: ${{ runner.temp }}/codegraph-comment.md + INLINE: ${{ runner.temp }}/codegraph-inline.json + WANT_INLINE: ${{ inputs.inline-comments }} + ANTHROPIC_API_KEY: ${{ inputs.anthropic-api-key }} + CODEGRAPH_AI_MODEL: ${{ inputs.ai-model }} + # A pull request from a fork is written by a stranger and cannot see secrets anyway: no model call for it. + SAME_REPO: ${{ github.event.pull_request.head.repo.full_name == github.repository }} + CODEGRAPH_PR_TITLE: ${{ github.event.pull_request.title }} + CODEGRAPH_PR_BODY: ${{ github.event.pull_request.body }} run: | if [ -z "$BASE_SHA" ]; then echo "::error::Run this action on pull_request events."; exit 1; fi - if ! git -C "$WORKSPACE" cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then - echo "::error::The base commit is not in the checkout. Use actions/checkout with fetch-depth: 0."; exit 1 - fi - npx tsx packages/cli/src/bin.ts impact --diff --range "$BASE_SHA...HEAD" --repo "$WORKSPACE" \ - --min-confidence "$MIN" --format markdown > "$OUT" + for sha in "$BASE_SHA" "$HEAD_SHA"; do + if ! git -C "$WORKSPACE" cat-file -e "$sha^{commit}" 2>/dev/null; then + echo "::error::A commit of the pull request is not in the checkout. Use actions/checkout with fetch-depth: 0."; exit 1 + fi + done + # Analyse the pull request's own commit, not the merge commit the checkout holds: GitHub numbers the lines of its diff + # (merge base to head) from that commit, and inline comments must land on those lines. + WT="$RUNNER_TEMP/codegraph-head" + git -C "$WORKSPACE" worktree add --quiet --detach "$WT" "$HEAD_SHA" > /dev/null 2>&1 + EXTRA="" + if [ "$WANT_INLINE" = "true" ]; then EXTRA="--inline-out $INLINE"; fi + if [ "$WANT_INLINE" = "all" ]; then EXTRA="--inline-out $INLINE --inline-hotspots"; fi + if [ -n "$ANTHROPIC_API_KEY" ] && [ "$SAME_REPO" = "true" ]; then EXTRA="$EXTRA --ai"; fi + npx tsx packages/cli/src/bin.ts impact --diff --range "$BASE_SHA...$HEAD_SHA" --repo "$WT" \ + --min-confidence "$MIN" --format markdown $EXTRA > "$OUT" + git -C "$WORKSPACE" worktree remove --force "$WT" || true # The headline number in the comment is "**N symbols in M files**"; no match means nothing was affected. - N=$(grep -oE '\*\*[0-9]+ symbols' "$OUT" | grep -oE '[0-9]+' | head -1 || true) + N=$(grep -oE '\*\*[0-9]+ symbols?' "$OUT" | grep -oE '[0-9]+' | head -1 || true) echo "symbols=${N:-0}" >> "$GITHUB_OUTPUT" echo "file=$OUT" >> "$GITHUB_OUTPUT" + if [ -s "$INLINE" ]; then echo "inline=$INLINE" >> "$GITHUB_OUTPUT"; fi - name: Post or update the comment if: steps.impact.outputs.file != '' uses: actions/github-script@v7 @@ -70,6 +100,18 @@ runs: core.warning(`Could not post the comment (${e.message}). Writing it to the job summary instead.`); await core.summary.addRaw(body).write(); } + - name: Leave line comments on the diff + if: steps.impact.outputs.inline != '' + uses: actions/github-script@v7 + env: + INLINE: ${{ steps.impact.outputs.inline }} + ACTION_PATH: ${{ github.action_path }} + with: + github-token: ${{ inputs.github-token }} + script: | + const post = require(`${process.env.ACTION_PATH}/action/post-review.cjs`); + const r = await post({ github, context, core, fs: require('fs'), file: process.env.INLINE }); + core.info(`Inline comments: ${r.posted} posted, ${r.kept} kept, ${r.removed} replaced.`); - name: Enforce the limit if: inputs.fail-above != '' && steps.impact.outputs.symbols != '' shell: bash diff --git a/action/post-review.cjs b/action/post-review.cjs new file mode 100644 index 0000000..b224433 --- /dev/null +++ b/action/post-review.cjs @@ -0,0 +1,65 @@ +// Posts CodeGraph's inline review comments on a pull request, replacing the ones it posted on an earlier push. +// Kept out of action.yml so it can be tested with a stand-in for the GitHub API. +const MARKER = ''; + +/** + * @param {{ github: any, context: any, core: any, fs: any, file: string }} env + */ +module.exports = async function postInline({ github, context, core, fs, file }) { + const { owner, repo } = context.repo; + const pr = context.payload.pull_request; + const pull_number = pr.number; + const wanted = JSON.parse(fs.readFileSync(file, 'utf8')); + + let existing; + try { + existing = await github.paginate(github.rest.pulls.listReviewComments, { owner, repo, pull_number, per_page: 100 }); + } catch (e) { + // A pull request from a fork gets a read-only token; the summary comment already carries the findings. + core.warning(`Could not read review comments (${e.message}); skipping inline comments.`); + return { posted: 0, kept: 0, removed: 0 }; + } + const mine = existing.filter((c) => c.body && c.body.includes(MARKER)); + const replied = new Set(existing.map((c) => c.in_reply_to_id).filter(Boolean)); + + // A comment that already says the same thing on the same line stays, so people's replies and resolutions are not lost. + const keep = new Set(); + const fresh = wanted.filter((w) => { + const same = mine.find((o) => !keep.has(o.id) && o.path === w.path && (o.line ?? o.original_line) === w.line && o.side === w.side && o.body.trim() === w.body.trim()); + if (same) keep.add(same.id); + return !same; + }); + let removed = 0; + for (const o of mine) { + if (keep.has(o.id) || replied.has(o.id)) continue; // never delete a thread someone answered + try { + await github.rest.pulls.deleteReviewComment({ owner, repo, comment_id: o.id }); + removed++; + } catch (e) { + core.warning(`Could not remove an old inline comment (${e.message}).`); + } + } + if (!fresh.length) return { posted: 0, kept: keep.size, removed }; + + const commit_id = pr.head.sha; + const comments = fresh.map(({ path, line, side, body }) => ({ path, line, side, body })); + try { + await github.rest.pulls.createReview({ owner, repo, pull_number, commit_id, event: 'COMMENT', comments }); + return { posted: comments.length, kept: keep.size, removed }; + } catch (e) { + // GitHub rejects the whole review if one anchor is not on the diff; try them one at a time and skip the refused ones. + core.warning(`Posting the review failed (${e.message}); trying the comments one by one.`); + } + let posted = 0; + for (const c of comments) { + try { + await github.rest.pulls.createReviewComment({ owner, repo, pull_number, commit_id, ...c }); + posted++; + } catch (e) { + core.warning(`Skipped an inline comment on ${c.path}:${c.line} (${e.message}).`); + } + } + return { posted, kept: keep.size, removed }; +}; + +module.exports.MARKER = MARKER; diff --git a/docs/AGENT-EVAL.md b/docs/AGENT-EVAL.md new file mode 100644 index 0000000..c782306 --- /dev/null +++ b/docs/AGENT-EVAL.md @@ -0,0 +1,58 @@ +# Does `check_change` catch a breaking edit before it is written? + +`codegraph_check_change` takes the new text of the files an agent is about to write and says what the edit would break. This +page measures that with a mutation test, using the TypeScript compiler as the ground truth. + +## Method (`scripts/eval/mutation-bench.mts`) + +For functions and methods that have callers, one edit at a time is made in memory (nothing is written): + +| edit | should it be flagged? | +|---|---| +| `remove` – delete the function | yes: calls are left behind | +| `add-param` – add a required parameter | yes: calls now pass too few arguments | +| `body-edit` – add a statement inside the body | no | +| `add-func` – add a new function | no | +| `remove-unused` – delete a function nobody calls | no | + +The compiler (a `ts.Program` over the repository's tsconfig, with the edited file served from memory) says whether each edit +breaks the build. The call sites the compiler binds to the declaration in the unmutated program (calls, `new`, JSX) are the +truth for *where*. `check_change` gets the same new file text. The baseline is `grep`: every line elsewhere that mentions the +name. + +## Result + +CodeGraph's own source (96 files; 20 edits per kind, seed 7; raw rows in `docs/eval/mutation-ts-self.json`): + +| edit | n | compiler: broken | `check_change`: breaking | caught | false alarms | call-site recall | call-site precision | grep recall | grep precision | +|---|---|---|---|---|---|---|---|---|---| +| remove | 20 | 20 | 20 | 100% | n/a | 100% | 100% | 100% | 34% | +| add-param | 20 | 17 | 15 | 88% | 0% | 86% | 100% | 100% | 34% | +| body-edit | 20 | 0 | 0 | n/a | 0% | | | | | +| add-func | 20 | 0 | 0 | n/a | 0% | | | | | + +Median time for a check, including the compiler run: about 3 s (the check itself is well under that). + +## What it misses, and what the numbers do not show + +- **React components.** All five `add-param` misses (and the two reported as `api-change`) are components used as + `