Skip to content

Pull request review: verdict, breaking changes, line comments, and an optional model reading - #10

Open
harsharajkumar-273 wants to merge 7 commits into
mainfrom
claude/practical-thompson-7w19xy
Open

harsharajkumar-273 wants to merge 7 commits into
mainfrom
claude/practical-thompson-7w19xy

Conversation

@harsharajkumar-273

@harsharajkumar-273 harsharajkumar-273 commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

What changes

The pull request comment says whether existing callers can break, shows the evidence, puts findings on the diff, and can optionally add a model's reading of what the change is for.

Facts (deterministic, no model):

  • A verdict at the top: 🔴 likely breaking (a removed or renamed function is still called, or a call site no longer fits a changed signature, naming the file and line) · 🟠 may change public API · 🟡 changes shared code · 🟢 contained or additive.
  • Which edit reaches whom, riskiest first; test command once with "N of M test files".
  • Line comments on the diff: a removed function that is still called goes on its deleted definition; a changed signature on the new signature, naming the calls that no longer fit; public API removed on the deleted lines. Replaced on each push; answered threads never deleted. The action now analyses the PR's own commit so line numbers match GitHub's diff.

New in this push, an optional reading by a model (the reason: the facts describe the graph, they cannot say what a change is for):

  • With anthropic-api-key set, the comment gains: what the change does in plain language, whether the diff matches the author's description (it will notice a change the description does not mention), and up to three things to check.
  • Off unless a key is set. Never runs for a pull request from a fork. When on, the source-file diff, the title and description, and the comment's facts are sent to the Anthropic API on every push (a few thousand tokens in, at most 700 out).
  • The verdict and facts go to the model as ground truth; the verdict does not depend on it.
  • Untrusted input: the title, description and diff are written by whoever opens the PR, so they are quoted inside tags the model is told never to obey; closing tags inside them are neutralised; the model's answer is stripped of markup, links, images and mentions and capped. A failing call never blocks the comment, and the key is never printed.

Evidence

  • Facts on real history: 132 real commits (Requests, Gin, Hono, this repo): no 🔴 or 🟠, and 0 inline comments by default on 118 of them. A rename and a new required argument injected into real Python, TypeScript and Go code are caught with the real call sites and anchored on the right diff line.
  • A correction made earlier: my first false-positive measurement analysed old commits against a clone at its newest state; I caught it and redid it correctly.
  • Typecheck, 185 tests, including: the exact request sent to the API (headers, body, key only as a header), a closing tag inside the diff cannot break out of its quotes, output cleaning, no call without a key or without --ai, the comment still posts when the API fails, and the key never appears in output or in the request body; end-to-end tests with real git commits for each verdict and each comment anchor.

Not verified

  • The model's answers have not been evaluated: that needs an API key, and I did not use one. I checked the request it would receive (it contains the diff, the facts and the description, so it can see a change the description omits), not what it would say. Please try it on a few of your own pull requests before relying on it.
  • The line-comment posting step has run on GitHub (it read the review comments and correctly posted nothing, since this PR has no finding) but has not yet created real line comments; that needs a PR with a finding.
  • Argument counts only (not names, order or types); calls only (not imports); every top-level Python function counts as public.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM

The comment now lists callers under the edited symbol that reaches
them, riskiest edit first, so a reviewer sees which change matters.
Top-level code is labelled as such, the test command is shown once
with how many of the project's test files it covers, and a change that
touches most of the suite says so.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
@vercel

vercel Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
codegraph Ready Ready Preview Oct 24, 2026 5:14am UTC

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 507d7be4-26f7-4f1b-abe2-5f02035e4168
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

CodeGraph: what this change can reach

Verdict: 🟡 changes shared code. No argument list changed incompatibly and nothing was removed, but the behaviour of code others call is different.

18 symbols in 14 files you did not edit could be affected by the 14 existing symbols this change edits (🟢 18 proven · 🟡 0 probable · 🔴 0 guessed by name).

Which edit reaches whom
  • packages/extractors/src/go.ts#extractGo.visit is reached by 16 in 14 files
    • 🟢 packages/extractors/src/extract.ts#extractFile, 🟢 packages/extractors/src/index-files.ts#indexFiles, 🟢 packages/extractors/test/browser-parity.test.ts (top-level code), 🟢 packages/extractors/test/go.test.ts (top-level code), and 12 more
  • packages/analysis/src/diff.ts#parseUnifiedDiff is reached by 2 in 1 file
    • 🟢 packages/cli/src/mcp.ts#buildTools, 🟢 packages/cli/src/mcp.ts#cmdMcp

Also: edits top-level code in 12 files; adds 88 new symbols.

12 of 14 test files to run

That is most of the suite: this change touches widely used code.

npx vitest run packages/analysis/test/analysis.test.ts packages/cli/test/ai.test.ts packages/cli/test/cli.test.ts packages/cli/test/inline.test.ts packages/extractors/test/browser-parity.test.ts packages/extractors/test/collisions.test.ts packages/extractors/test/go.test.ts packages/extractors/test/python.test.ts packages/extractors/test/typescript.test.ts packages/extractors/test/variables.test.ts packages/web/test/model.test.ts packages/web/test/sources.test.ts

Static analysis of calls and imports (TypeScript, JavaScript, Python, Go). 🟡 and 🔴 links are name matches, so check them before relying on them. Dynamic calls are invisible to it. CodeGraph

The first real run showed '1 symbols in 1 files', '1 of 11 test file',
and a list jammed against the code fence. The action's headline pattern
now accepts the singular so the fail-above limit still reads it.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
The pull request comment now indexes the base commit too and lists any
function that existed before, is gone now, and has a surviving caller
that still calls the old name: a rename that missed a caller. A name the
caller already called without resolving in the base (an external get)
is not reported, which removes the main way this check could cry wolf.

On 132 real commits from five repositories it flagged nothing, and on a
rename injected into Requests, Hono and Gin it named the real callers.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
@harsharajkumar-273 harsharajkumar-273 changed the title Make pull request comments show which edit reaches whom Pull request comments: show which edit reaches whom, and catch renames that missed a caller Oct 23, 2026
The comment now opens with whether existing callers can fail: likely
breaking, may change public API, changes shared code, or contained or
additive. A function whose argument counts changed is checked against
how many arguments each call site passes, so the comment names the calls
that no longer fit instead of guessing. Argument counts are recorded for
TypeScript, JavaScript, Python and Go.

Symbols that only moved to another file are no longer reported as
removed, symbols that the base commit lacks are new even when git merges
them into an edited hunk, and leading-underscore Python names are not
public.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
@harsharajkumar-273 harsharajkumar-273 changed the title Pull request comments: show which edit reaches whom, and catch renames that missed a caller Pull request comments: a verdict, which edit reaches whom, and breaking changes caught Oct 23, 2026
A removed function that is still called is commented on its deleted
definition, since the caller that was missed sits in a file the pull
request did not touch and GitHub only takes comments on diff lines. A
changed signature is commented on the new signature, naming the calls
that no longer fit. Public API removed is commented on the deleted
lines. Comments are replaced on each push and left alone once someone
has replied.

The action now analyses the pull request's own commit in a worktree
instead of the merge commit, so its line numbers match GitHub's diff.
A note on edits to widely used code is opt-in (inline-comments: all):
on 118 real commits it would have fired on about one in seven, which is
noise, while the default fired on none.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
@harsharajkumar-273 harsharajkumar-273 changed the title Pull request comments: a verdict, which edit reaches whom, and breaking changes caught Pull request review: a verdict, breaking changes caught, and line comments on the diff Oct 23, 2026
With an Anthropic API key the comment gains a short reading: what the
change does in plain language, whether the diff matches the author's
description, and what to check. The deterministic verdict and facts go
to the model as ground truth and do not depend on it.

Off unless a key is set, and never used 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, closing tags inside them are neutralised, and the answer is
stripped of markup, links and mentions before it is shown. A failing
call never blocks the comment, and the key is never printed.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ENHRK3kgqcVwqwyjswRViM
@harsharajkumar-273 harsharajkumar-273 changed the title Pull request review: a verdict, breaking changes caught, and line comments on the diff Pull request review: verdict, breaking changes, line comments, and an optional model reading Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants