feat(analyzer): add semantic diff understanding for TS/JS#25
Conversation
- AST-based analysis using ts-morph - Detects function renames, API signature changes, interface changes - Integrates with summarizer and commit generator - Includes unit tests and timeout protection Closes nirvik34#7
|
@PiyushRai98 is attempting to deploy a commit to the nirvik34's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Warning
|
| Layer / File(s) | Summary |
|---|---|
Semantic Types & Dependencies src/analyzer/semanticTypes.ts, package.json |
New semantic event/result types; adds ts-morph dependency and bumps @types/node. |
Git Content Extraction src/git/getFileContent.ts |
getStagedFileContent and getWorkingFileContent read staged and working-tree file contents and return null for missing/deleted files. |
Semantic Change Detectors src/analyzer/semanticAnalyzer.ts |
Function-keying and AST-based detectors for function_rename, api_signature_change, and interface_change; per-file ts-morph projects, timeout enforcement, and aggregated analyzeSemanticChanges entrypoint with per-file error logging. |
Semantic Analysis Tests src/analyzer/__tests__/semanticAnalyzer.test.ts |
Vitest tests mocking git helpers validating rename, signature-change, and no-change detection scenarios. |
Summarizer & Commit Message Integration src/analyzer/summarizer.ts, src/generator/commitGenerator.ts |
Adds generateConventionalCommitMessage and FileInfo type; enhances subject/body generation using prioritized semantic events and extends generateCommitMessage to accept semanticEvents. |
Main Flow Integration src/index.ts |
Adds verbose CLI option; runs semantic analysis (when options.ai !== false), logs when verbose, and passes semanticEvents into commit message generation while preserving existing fallbacks and AI gating. |
Sequence Diagram(s)
sequenceDiagram
participant run as run()
participant analyzeSemanticChanges
participant getStagedFileContent
participant getWorkingFileContent
participant tsmorph as ts-morph
participant generateSummary
participant generateCommitMessage
run->>analyzeSemanticChanges: file paths
analyzeSemanticChanges->>getStagedFileContent: read staged content
analyzeSemanticChanges->>getWorkingFileContent: read working content
analyzeSemanticChanges->>tsmorph: create projects & parse ASTs
tsmorph-->>analyzeSemanticChanges: semantic events
analyzeSemanticChanges-->>run: events, duration, skipped
run->>generateSummary: files, semanticEvents
generateSummary-->>run: conventional commit subject
run->>generateCommitMessage: type, scope, files, semanticEvents
generateCommitMessage-->>run: full commit message
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- nirvik34/gitbun#13: Both PRs modify
src/analyzer/summarizer.tsbehavior and signature, and both affect howsrc/index.tsfeeds data into commit message generation.
Suggested labels
level: advanced, type: feature, type:testing, quality:clean, gssoc:approved
Suggested reviewers
- 2PieRadian
Poem
🐰 With nimble hops I parse each tree,
I name the renames and count each arity,
I stitch events into a tidy line,
A hop, a pipe, a commit—oh fine!
🥕
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat(analyzer): add semantic diff understanding for TS/JS' accurately and concisely summarizes the main feature: adding semantic diff analysis for TypeScript/JavaScript files using AST-based approach. |
| Linked Issues check | ✅ Passed | The PR successfully implements all core requirements from issue #7: AST-based semantic analysis using ts-morph, detection of function renames, API signature changes, and interface changes, integration into commit generation, and performance protection via 500ms timeout. |
| Out of Scope Changes check | ✅ Passed | All changes are directly aligned with issue #7 objectives. Package dependency addition (ts-morph) supports the semantic analysis feature, and CLI enhancements integrate semantic results into the commit generation workflow as required. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
This PR adds AST-based “semantic diff” analysis for TypeScript/JavaScript changes and threads the detected semantic events into Gitbun’s summarization and commit-message generation flow, backed by a new ts-morph dependency and unit tests.
Changes:
- Introduces a
semanticAnalyzer(ts-morph) +semanticTypesto detect function renames, signature changes, and interface/type changes. - Integrates semantic events into the CLI pipeline and commit message generation.
- Adds git content helpers and Vitest coverage for the semantic analyzer.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Runs semantic analysis and passes events into summary + commit generation. |
| src/git/getFileContent.ts | Adds helpers to fetch staged/working file contents for analysis. |
| src/generator/commitGenerator.ts | Uses semantic events to generate higher-level commit descriptions. |
| src/analyzer/summarizer.ts | Expands summarizer to incorporate semantic events (and changes output format). |
| src/analyzer/semanticTypes.ts | Defines semantic event/result types. |
| src/analyzer/semanticAnalyzer.ts | Implements AST-based change detection with timeout fallback. |
| src/analyzer/tests/semanticAnalyzer.test.ts | Adds unit tests for rename/signature/no-change detection. |
| package.json | Adds ts-morph dependency; bumps @types/node. |
| package-lock.json | Lockfile updates for new dependencies/version bump alignment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/analyzer/__tests__/semanticAnalyzer.test.ts (1)
11-57: ⚡ Quick winAdd coverage for timeout skip and interface/type-change scenarios.
Current suite misses two core paths of this feature: timeout fallback (
skipped: true) andinterface_changeemission.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/analyzer/__tests__/semanticAnalyzer.test.ts` around lines 11 - 57, Add two new test cases in src/analyzer/__tests__/semanticAnalyzer.test.ts: one that simulates the analysis timing out and returns { skipped: true } (mock fileContent.getStagedFileContent and getWorkingFileContent as needed and assert result.skipped === true) to cover the timeout fallback path, and another that supplies an oldContent/newContent pair that changes an interface or type declaration (e.g., interface Foo { a: number } -> interface Foo { a: number; b: string } or type alias change), call analyzeSemanticChanges(["src/test.ts"]) and assert an event with type "interface_change" (or "type_change") is emitted and its details reflect the change; use the existing test patterns and vi.mocked(fileContent.getStagedFileContent) / vi.mocked(fileContent.getWorkingFileContent) to stub contents and locate the behavior around analyzeSemanticChanges in the test file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/analyzer/summarizer.ts`:
- Around line 62-64: The current check inside the diff loop erroneously flips
hasDeletionsOnly to false for any change because it tests (file.additions > 0 ||
file.deletions > 0); update the logic so hasDeletionsOnly is only set to false
when there are additions (file.additions > 0). In other words, in the loop that
iterates over file diffs (the code referencing variable file and
hasDeletionsOnly in summarizer.ts) change the condition to only clear
hasDeletionsOnly on additions so deletion-only files (additions === 0 &&
deletions > 0) remain classified as deletions-only.
In `@src/git/getFileContent.ts`:
- Around line 11-31: The git and file-read calls in getStagedFileContent and
getWorkingFileContent are vulnerable to shell injection because they interpolate
filePath into a shell command; change getStagedFileContent to call execFile (or
execFileAsync equivalent) with "git" and args ["show", `:${filePath}`] so the
path is passed as an argument rather than interpolated, preserve the existing
error handling logic for "bad revision"/"does not exist" and return stdout as
before; change getWorkingFileContent to use fs.promises.readFile (or a
readFileAsync helper) with the filePath argument and return the file buffer as a
UTF-8 string (or null on read errors consistent with current behavior) instead
of using execAsync("cat ..."). Ensure any helpers referenced (execAsync ->
execFileAsync) are adjusted accordingly.
In `@src/index.ts`:
- Around line 60-63: analyzeSemanticChanges is called directly and can throw,
causing commit generation to abort instead of falling back to the diff-based
path; wrap the call to analyzeSemanticChanges(filePaths) in a try/catch around
the block that uses enrichedFiles and semanticResult so that any thrown error is
caught, log the error (or record it) and set semanticResult to a "skipped" state
(or equivalent) to allow the existing diff-based commit generation path to
continue; update references around analyzeSemanticChanges and semanticResult so
downstream checks like semanticResult.skipped and semanticResult.events.length
still behave correctly when an exception occurred.
---
Nitpick comments:
In `@src/analyzer/__tests__/semanticAnalyzer.test.ts`:
- Around line 11-57: Add two new test cases in
src/analyzer/__tests__/semanticAnalyzer.test.ts: one that simulates the analysis
timing out and returns { skipped: true } (mock fileContent.getStagedFileContent
and getWorkingFileContent as needed and assert result.skipped === true) to cover
the timeout fallback path, and another that supplies an oldContent/newContent
pair that changes an interface or type declaration (e.g., interface Foo { a:
number } -> interface Foo { a: number; b: string } or type alias change), call
analyzeSemanticChanges(["src/test.ts"]) and assert an event with type
"interface_change" (or "type_change") is emitted and its details reflect the
change; use the existing test patterns and
vi.mocked(fileContent.getStagedFileContent) /
vi.mocked(fileContent.getWorkingFileContent) to stub contents and locate the
behavior around analyzeSemanticChanges in the test file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac1d532a-31d0-4276-aa98-c96a91a97a11
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonsrc/analyzer/__tests__/semanticAnalyzer.test.tssrc/analyzer/semanticAnalyzer.tssrc/analyzer/semanticTypes.tssrc/analyzer/summarizer.tssrc/generator/commitGenerator.tssrc/git/getFileContent.tssrc/index.ts
PiyushRai98
force-pushed
the
feat/semantic-diff-understanding
branch
from
8d20258 to
14ba674
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/analyzer/semanticAnalyzer.ts (1)
35-40:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftClass and object method changes are not detected.
getFunctions()captures only top-level function declarations, so class method renames, signature changes, and object method modifications remain invisible to the semantic analyzer. This gap affects common TypeScript/JavaScript refactoring patterns where methods are modified within classes or objects.Also applies to: 78-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/analyzer/semanticAnalyzer.ts` around lines 35 - 40, The analyzer currently only uses oldSource.getFunctions() and newSource.getFunctions(), which misses class methods and object literal/variable-assigned functions; update the collection logic that builds oldFunctions/newFunctions to also traverse each source (oldSource and newSource) and include class methods (use getClasses() then getMethods()), interface/type method signatures if needed, object literal methods and property assignments whose initializer is a FunctionExpression/ArrowFunction, and variable declarations with function/arrow initializers; ensure you collect the same set for both old and new so comparisons detect method renames/signature changes (update any helper that runs the comparison logic referenced by oldFunctions/newFunctions and mirror this at the other occurrence noted near the later block).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/analyzer/semanticAnalyzer.ts`:
- Around line 148-156: The current diff logic only computes added/removed props
by name using oldInt/newInt.getProperties().map(...), so properties that remain
but change type or optionality are missed; after building oldProps and newProps,
iterate over props present in both (match by name) and compare their type and
optionality fields from the structures (e.g., structure.type,
structure.hasQuestionToken or structure.hasQuestionToken/optional flag), collect
these into a "changed" list and emit the appropriate change events (similar to
added/removed) for properties whose type or optionality differ between newProps
and oldProps; update any downstream handling to accept the new "changed"
entries.
In `@src/analyzer/summarizer.ts`:
- Around line 90-92: The addedFiles filter in summarizer.ts currently only
matches .ts and .js, causing .tsx/.jsx additions to be excluded from subject
generation; update the files.filter used to build addedFiles (the const
addedFiles = files.filter(...) expression) to include .tsx and .jsx (e.g. adjust
the regex to match tsx|tsx|js|jsx variants, optionally with case-insensitivity)
so generateSubject will consider those file types when producing "add …"
subjects.
---
Outside diff comments:
In `@src/analyzer/semanticAnalyzer.ts`:
- Around line 35-40: The analyzer currently only uses oldSource.getFunctions()
and newSource.getFunctions(), which misses class methods and object
literal/variable-assigned functions; update the collection logic that builds
oldFunctions/newFunctions to also traverse each source (oldSource and newSource)
and include class methods (use getClasses() then getMethods()), interface/type
method signatures if needed, object literal methods and property assignments
whose initializer is a FunctionExpression/ArrowFunction, and variable
declarations with function/arrow initializers; ensure you collect the same set
for both old and new so comparisons detect method renames/signature changes
(update any helper that runs the comparison logic referenced by
oldFunctions/newFunctions and mirror this at the other occurrence noted near the
later block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c239da84-4935-4174-91e7-b4f015413e15
📒 Files selected for processing (6)
package.jsonsrc/analyzer/semanticAnalyzer.tssrc/analyzer/summarizer.tssrc/generator/commitGenerator.tssrc/git/getFileContent.tssrc/index.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/analyzer/semanticAnalyzer.ts (1)
168-191:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate
interface_changeevents emitted when properties are added or removed.The new block (lines 168-179) correctly handles all three cases (
added,removed,changed), but the old block (lines 181-191) was not removed. Whenadded.length || removed.lengthis true, twointerface_changeevents are pushed for the same interface.🐛 Proposed fix: Remove the redundant old block
details: { added: added.map((p) => p.name), removed: removed.map((p) => p.name), changed, }, }); } - - if (added.length || removed.length) { - events.push({ - type: "interface_change", - file: filePath, - entityName: name, - details: { - added: added.map((p) => p.name), - removed: removed.map((p) => p.name), - }, - }); - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/analyzer/semanticAnalyzer.ts` around lines 168 - 191, The code is emitting duplicate "interface_change" events: keep the new consolidated events.push block that includes added, removed, and changed (which references added, removed, changed, filePath, name) and remove the older redundant events.push block that only handled added/removed; delete the second events.push (the one that checks if (added.length || removed.length) and pushes type: "interface_change" with only added/removed) so a single event is emitted per interface change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/analyzer/semanticAnalyzer.ts`:
- Around line 168-191: The code is emitting duplicate "interface_change" events:
keep the new consolidated events.push block that includes added, removed, and
changed (which references added, removed, changed, filePath, name) and remove
the older redundant events.push block that only handled added/removed; delete
the second events.push (the one that checks if (added.length || removed.length)
and pushes type: "interface_change" with only added/removed) so a single event
is emitted per interface change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4f391d7-5c2e-406a-ad09-552c9a501884
📒 Files selected for processing (1)
src/analyzer/semanticAnalyzer.ts
Removed event logging for interface changes in semantic analysis.
|
Addressed the review comments, resolved merge conflicts with main, and fixed the failing test cases caused by the semantic diff integration changes. Re-ran the test suite and verified the analyzer pipeline is working as expected. |
|
🎉 This PR is included in version 1.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nirvik34
added
type: feature
|
Hi @nirvik34, the PR currently shows only the base +50 points despite having advanced, feature, and quality: exceptional labels. Could you please check if the bonus points are pending sync? |
|
@PiyushRai98 i have use the labels appropriately as you have seen in the PR. Maybe there is some syncing issue with the site.. its unable to scrap the data properly.. wait for some timee |
Closes #7
Description
This PR introduces semantic diff understanding to Gitbun’s commit message generation pipeline. Instead of relying solely on file names and raw diffs, the analyzer now compares AST structures of TypeScript/JavaScript files to detect high-level code changes such as function renames, API signature modifications, and interface/type changes. The semantic events are seamlessly integrated into the existing rule‑based summarizer and commit generator, resulting in more accurate and human‑readable commit messages. Performance is safeguarded by a 500ms timeout and graceful fallback to the original diff‑based logic.
Type of change
GSSoC '26 Contribution Details
Please select only one difficulty level that was assigned to you in the issue:
level:beginner
level:intermediate
level:advanced
level:critical
I have been assigned to this issue by a maintainer. (PRs without prior assignment will not count toward GSSoC).
How Has This Been Tested?
npm testpasses locallynpm run lintpasses without errorsnpm run devand verified the outputChecklist:
Summary by CodeRabbit
New Features
Dependencies
Tests