Skip to content

Commit 2a6d36f

Browse files
committed
refactor(review): simplify review flow and remove intent analysis #453
Streamline the code review process by removing commit intent analysis and related steps. Now, only technical review and fix generation are performed, improving maintainability and reducing complexity.
1 parent 0c5efc6 commit 2a6d36f

1 file changed

Lines changed: 69 additions & 265 deletions

File tree

mpp-ui/src/jsMain/typescript/modes/ReviewMode.ts

Lines changed: 69 additions & 265 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,6 @@ export async function runReview(
113113
}
114114
}
115115

116-
// ===== STEP 5: Extract Commit Message =====
117-
console.log(semanticChalk.info('📝 Extracting commit information...'));
118-
const { commitMessage, commitId, repoUrl } = await extractCommitInfo(projectPath, options);
119-
console.log();
120-
121116
// Create code review agent
122117
const reviewAgent = new KotlinCC.unitmesh.agent.JsCodeReviewAgent(
123118
projectPath,
@@ -128,187 +123,85 @@ export async function runReview(
128123
null
129124
);
130125

131-
// ===== STEP 6: Intent Analysis (if commit message exists) =====
132-
let intentAnalysisOutput = '';
133-
let mermaidDiagram: string | null = null;
134-
135-
if (commitMessage && commitMessage.trim().length > 10) {
136-
console.log(semanticChalk.info('🎯 Analyzing commit intent...'));
137-
console.log(semanticChalk.muted(`📄 Commit: ${commitId || 'HEAD'}`));
138-
console.log(semanticChalk.muted(`💬 Message: ${commitMessage.substring(0, 80)}${commitMessage.length > 80 ? '...' : ''}`));
139-
console.log();
140-
141-
const llmStartTime = Date.now();
142-
143-
try {
144-
// Build code changes map from diff content
145-
const codeChangesMap = buildCodeChangesMap(diffContent, filePaths, codeContent);
146-
147-
console.log(semanticChalk.info('⚡ Streaming intent analysis...'));
148-
console.log();
149-
150-
// Call executeTask with analyzeIntent enabled
151-
const intentResult = await reviewAgent.executeTask(
152-
'COMPREHENSIVE',
153-
filePaths,
154-
codeChangesMap,
155-
{},
156-
'', // diffContext - not needed for intent analysis
157-
commitMessage,
158-
commitId,
159-
repoUrl,
160-
'', // issueToken - could be added as an option
161-
true, // useTools = true for tool-driven approach
162-
true, // analyzeIntent = true
163-
'EN',
164-
(chunk: string) => {
165-
// process.stdout.write(chunk);
166-
}
167-
);
168-
169-
intentAnalysisOutput = intentResult.content;
170-
mermaidDiagram = intentResult.mermaidDiagram || null;
171-
172-
const llmDuration = Date.now() - llmStartTime;
173-
174-
console.log();
175-
console.log();
176-
console.log(semanticChalk.success('✅ Intent analysis complete!'));
177-
console.log(semanticChalk.muted(`⏱️ Time: ${llmDuration}ms`));
178-
179-
// Display mermaid diagram if present
180-
if (mermaidDiagram) {
181-
console.log();
182-
console.log(semanticChalk.info('📊 Intent Flow Diagram:'));
183-
console.log();
184-
console.log(semanticChalk.muted('```mermaid'));
185-
console.log(mermaidDiagram);
186-
console.log(semanticChalk.muted('```'));
187-
}
188-
189-
console.log();
190-
console.log(semanticChalk.muted('─'.repeat(80)));
191-
console.log();
192-
} catch (error: any) {
193-
console.log(semanticChalk.warning(`⚠️ Intent analysis failed: ${error.message}`));
194-
console.log(semanticChalk.muted('Continuing with technical review...'));
195-
console.log();
196-
}
197-
}
198-
199-
// ===== STEP 7: Technical Code Review (if intent analysis was done) =====
200-
let technicalReviewOutput = '';
201-
202-
if (intentAnalysisOutput) {
203-
console.log(semanticChalk.info('🔧 Performing technical code review...'));
204-
console.log();
205-
206-
// Build diff context
207-
const diffContext = diffContent.length > 2000
208-
? `\n## What Changed\n\`\`\`diff\n${diffContent.substring(0, 2000)}...\n\`\`\``
209-
: `\n## What Changed\n\`\`\`diff\n${diffContent}\n\`\`\``;
126+
// ===== STEP 5: Execute Review Task (like CodeReviewViewModel) =====
127+
console.log(semanticChalk.info('🧠 Starting code review analysis...'));
128+
console.log();
210129

211-
const promptLength = JSON.stringify(codeContent).length + JSON.stringify(lintResults).length + diffContext.length;
212-
console.log(semanticChalk.muted(`📊 Prompt: ${promptLength} chars (~${Math.floor(promptLength / 4)} tokens)`));
213-
console.log(semanticChalk.info('⚡ Streaming technical review...'));
214-
console.log();
130+
const llmStartTime = Date.now();
215131

216-
const llmStartTime = Date.now();
217-
const technicalResult = await reviewAgent.executeTask(
132+
try {
133+
const analysisResult = await reviewAgent.executeTask(
218134
reviewType,
219135
filePaths,
220136
codeContent,
221137
lintResults,
222-
diffContext,
223-
'', // commitMessage
224-
'', // commitId
225-
'', // repoUrl
226-
'', // issueToken
227-
false, // useTools = false for data-driven mode
228-
false, // analyzeIntent = false
229-
'EN',
230-
(chunk: string) => {
231-
process.stdout.write(chunk);
232-
}
138+
diffContent,
139+
'', // commitMessage
140+
'', // commitId
141+
'', // repoUrl
142+
'', // issueToken
143+
false, // useTools
144+
false, // analyzeIntent
145+
'EN', // language
146+
// renderer ? (chunk: string) => renderer.renderLLMResponseChunk(chunk) : undefined
233147
);
234-
technicalReviewOutput = technicalResult.content;
148+
149+
const analysisContent = analysisResult.content;
235150

236151
const llmDuration = Date.now() - llmStartTime;
237152
console.log();
238-
console.log();
239-
console.log(semanticChalk.success('✅ Technical review complete!'));
153+
console.log(semanticChalk.success('✅ Code review analysis complete!'));
240154
console.log(semanticChalk.muted(`⏱️ Time: ${llmDuration}ms`));
241-
} else {
242-
// No commit message, just do technical review
243-
console.log(semanticChalk.info('🤖 Analyzing with AI...'));
244-
245-
const diffContext = diffContent.length > 2000
246-
? `\n## What Changed\n\`\`\`diff\n${diffContent.substring(0, 2000)}...\n\`\`\``
247-
: `\n## What Changed\n\`\`\`diff\n${diffContent}\n\`\`\``;
155+
console.log();
248156

249-
const promptLength = JSON.stringify(codeContent).length + JSON.stringify(lintResults).length + diffContext.length;
250-
console.log(semanticChalk.muted(`📊 Prompt: ${promptLength} chars (~${Math.floor(promptLength / 4)} tokens)`));
251-
console.log(semanticChalk.info('⚡ Streaming AI response...'));
157+
// ===== STEP 6: Generate Fixes (like CodeReviewViewModel.generateFixes) =====
158+
console.log(semanticChalk.info('🔧 Generating actionable fixes...'));
252159
console.log();
253160

254-
const llmStartTime = Date.now();
161+
const fixStartTime = Date.now();
255162

256-
const technicalResult = await reviewAgent.executeTask(
257-
reviewType,
258-
filePaths,
163+
const fixOutput = await reviewAgent.generateFixes(
259164
codeContent,
260-
lintResults,
261-
diffContext,
262-
'', // commitMessage
263-
'', // commitId
264-
'', // repoUrl
265-
'', // issueToken
266-
false, // useTools = false for data-driven mode
267-
false, // analyzeIntent = false
165+
convertLintResultsToArray(lintResults, filePaths),
166+
analysisContent,
268167
'EN',
269-
(chunk: string) => {
270-
// process.stdout.write(chunk);
271-
}
168+
// renderer ? (chunk: string) => renderer.renderLLMResponseChunk(chunk) : undefined
272169
);
273-
technicalReviewOutput = technicalResult.content;
274170

275-
const llmDuration = Date.now() - llmStartTime;
171+
const fixDuration = Date.now() - fixStartTime;
276172
console.log();
277173
console.log();
278-
console.log(semanticChalk.success('✅ Code review complete!'));
279-
console.log(semanticChalk.muted(`⏱️ Time: ${llmDuration}ms`));
280-
}
174+
console.log(semanticChalk.success('✅ Fix generation complete!'));
175+
console.log(semanticChalk.muted(`⏱️ Time: ${fixDuration}ms`));
176+
console.log();
281177

282-
const totalDuration = Date.now() - startTime;
178+
const totalDuration = Date.now() - startTime;
179+
console.log(semanticChalk.success('✅ Complete review finished!'));
180+
console.log(semanticChalk.muted(`⏱️ Total: ${totalDuration}ms`));
181+
console.log();
283182

284-
// Combine outputs
285-
const analysisOutput = intentAnalysisOutput
286-
? `# 🎯 Intent Analysis\n\n${intentAnalysisOutput}\n\n---\n\n# 🔧 Technical Review\n\n${technicalReviewOutput}`
287-
: technicalReviewOutput;
183+
// Parse findings from analysis output
184+
const findings = parseMarkdownFindings(analysisContent);
288185

289-
console.log();
290-
console.log();
291-
console.log(semanticChalk.success('✅ Complete review finished!'));
292-
console.log(semanticChalk.muted(`⏱️ Total: ${totalDuration}ms`));
293-
console.log();
186+
if (findings.length > 0) {
187+
displayKotlinFindings(findings);
188+
} else {
189+
console.log(semanticChalk.success('✨ No significant issues found! Code looks good.'));
190+
console.log();
191+
}
294192

295-
// Parse findings from technical review only (not from intent analysis)
296-
const findings = parseMarkdownFindings(technicalReviewOutput || analysisOutput);
193+
return {
194+
success: true,
195+
message: analysisContent,
196+
findings: findings,
197+
analysisOutput: analysisContent
198+
};
297199

298-
if (findings.length > 0) {
299-
displayKotlinFindings(findings);
300-
} else {
301-
console.log(semanticChalk.success('✨ No significant issues found! Code looks good.'));
302-
console.log();
200+
} catch (error: any) {
201+
console.error(semanticChalk.error(`Analysis failed: ${error.message}`));
202+
throw error;
303203
}
304204

305-
return {
306-
success: true,
307-
message: analysisOutput,
308-
findings: findings,
309-
analysisOutput: analysisOutput
310-
};
311-
312205
} catch (error: any) {
313206
console.error(semanticChalk.error(`Review failed: ${error.message}`));
314207
console.error(error.stack);
@@ -516,117 +409,28 @@ function parseMarkdownFindings(markdown: string): EnhancedReviewFinding[] {
516409
}
517410

518411
/**
519-
* Extract commit information (message, ID, repo URL)
412+
* Convert lint results from Record to Array format for generateFixes
520413
*/
521-
async function extractCommitInfo(
522-
projectPath: string,
523-
options: ReviewOptions
524-
): Promise<{ commitMessage: string; commitId: string; repoUrl: string }> {
525-
try {
526-
const { execSync } = await import('child_process');
527-
528-
// Get commit ID
529-
let commitId = options.commitHash || '';
530-
if (!commitId) {
531-
try {
532-
commitId = execSync('git rev-parse HEAD', {
533-
cwd: projectPath,
534-
encoding: 'utf-8'
535-
}).trim();
536-
} catch {
537-
commitId = 'HEAD';
538-
}
539-
}
540-
541-
// Get commit message
542-
let commitMessage = '';
543-
try {
544-
const command = options.commitHash
545-
? `git log -1 --format=%B ${options.commitHash}`
546-
: 'git log -1 --format=%B HEAD';
547-
548-
commitMessage = execSync(command, {
549-
cwd: projectPath,
550-
encoding: 'utf-8'
551-
}).trim();
552-
} catch (error: any) {
553-
console.log(semanticChalk.muted(`Unable to get commit message: ${error.message}`));
554-
}
555-
556-
// Get repo URL
557-
let repoUrl = '';
558-
try {
559-
const remoteUrl = execSync('git config --get remote.origin.url', {
560-
cwd: projectPath,
561-
encoding: 'utf-8'
562-
}).trim();
563-
564-
// Convert SSH URL to HTTPS if needed
565-
if (remoteUrl.startsWith('git@github.com:')) {
566-
repoUrl = remoteUrl.replace('git@github.com:', 'https://github.com/').replace(/\.git$/, '');
567-
} else {
568-
repoUrl = remoteUrl.replace(/\.git$/, '');
569-
}
570-
} catch {
571-
// Repo URL is optional
572-
}
573-
574-
return { commitMessage, commitId, repoUrl };
575-
} catch (error: any) {
576-
console.log(semanticChalk.muted(`Unable to extract commit info: ${error.message}`));
577-
return { commitMessage: '', commitId: '', repoUrl: '' };
578-
}
579-
}
580-
581-
/**
582-
* Build code changes map from diff content
583-
* Returns a map of file paths to their diff content
584-
*/
585-
function buildCodeChangesMap(
586-
diffContent: string,
587-
filePaths: string[],
588-
codeContent: Record<string, string>
589-
): Record<string, string> {
590-
const codeChangesMap: Record<string, string> = {};
591-
592-
// Try to extract per-file diffs
593-
const lines = diffContent.split('\n');
594-
let currentFile = '';
595-
let currentDiff: string[] = [];
596-
597-
for (const line of lines) {
598-
if (line.startsWith('diff --git ')) {
599-
// Save previous file's diff
600-
if (currentFile && currentDiff.length > 0) {
601-
codeChangesMap[currentFile] = currentDiff.join('\n');
602-
}
603-
604-
// Start new file
605-
const match = line.match(/diff --git a\/(.*?) b\/(.*?)$/);
606-
if (match) {
607-
currentFile = match[2];
608-
currentDiff = [line];
609-
}
610-
} else if (currentFile) {
611-
currentDiff.push(line);
612-
}
613-
}
614-
615-
// Save last file's diff
616-
if (currentFile && currentDiff.length > 0) {
617-
codeChangesMap[currentFile] = currentDiff.join('\n');
618-
}
619-
620-
// If no per-file diffs were extracted, use full code content as fallback
621-
if (Object.keys(codeChangesMap).length === 0) {
622-
filePaths.forEach(filePath => {
623-
if (codeContent[filePath]) {
624-
codeChangesMap[filePath] = codeContent[filePath];
625-
}
414+
function convertLintResultsToArray(
415+
lintResults: Record<string, string>,
416+
filePaths: string[]
417+
): Array<any> {
418+
const lintArray: Array<any> = [];
419+
420+
for (const [linterName, output] of Object.entries(lintResults)) {
421+
// Create a simple lint result structure
422+
// This matches the expected JsLintFileResult format
423+
lintArray.push({
424+
filePath: filePaths[0] || 'unknown',
425+
linterName: linterName,
426+
errorCount: 0,
427+
warningCount: 0,
428+
infoCount: 0,
429+
issues: []
626430
});
627431
}
628432

629-
return codeChangesMap;
433+
return lintArray;
630434
}
631435

632436
/**

0 commit comments

Comments
 (0)