Skip to content

Commit cb8f572

Browse files
authored
Merge pull request #85 from DGouron/fix/followup-score-boost
fix(tracking): remove averageScore, use latestScore for MR cards
2 parents b113d74 + 3c8aefa commit cb8f572

File tree

9 files changed

+57
-34
lines changed

9 files changed

+57
-34
lines changed

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ coverage/
3737
logs/security/
3838
.claude/skills
3939
.claude/memory
40-
.claude/backlog
4140
CLAUDE.md
4241

4342
# MCP config (auto-generated by ensureProjectMcpConfig)

docs/architecture/mcp/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,4 @@ yarn add @modelcontextprotocol/sdk
156156

157157
## Next Steps
158158

159-
See tickets in `.claude/backlog/tickets/`
159+
See [GitHub Issues](https://github.com/DGouron/review-flow/issues) and the [ReviewFlow Roadmap](https://github.com/users/DGouron/projects/3) project board.

src/entities/tracking/trackedMr.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export interface TrackedMr {
3131
totalWarnings: number;
3232
totalSuggestions: number;
3333
totalDurationMs: number;
34-
averageScore: number | null;
3534
latestScore: number | null;
3635

3736
autoFollowup: boolean;

src/frameworks/claude/claudeInvoker.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -490,18 +490,19 @@ export async function invokeClaudeReview(
490490
outputLength: stdout.length,
491491
});
492492

493-
// Save review statistics
494-
try {
495-
// Look up assignor from MR tracking
496-
const mrId = `${job.platform}-${job.projectPath}-${job.mrNumber}`;
497-
const trackingGateway = new FileSystemReviewRequestTrackingGateway(new ProjectStatsCalculator());
498-
const mrDetails = trackingGateway.getById(job.localPath, mrId);
499-
const assignedBy = mrDetails?.assignment?.username;
500-
501-
const reviewStats = addReviewStats(job.localPath, job.mrNumber, durationMs, stdout, assignedBy);
502-
logger.info({ reviewStats }, 'Stats de review enregistrées');
503-
} catch (statsError) {
504-
logger.warn({ error: statsError }, 'Erreur lors de l\'enregistrement des stats');
493+
// Save review statistics (followups are not counted as reviews)
494+
if (job.jobType !== 'followup') {
495+
try {
496+
const mrId = `${job.platform}-${job.projectPath}-${job.mrNumber}`;
497+
const trackingGateway = new FileSystemReviewRequestTrackingGateway(new ProjectStatsCalculator());
498+
const mrDetails = trackingGateway.getById(job.localPath, mrId);
499+
const assignedBy = mrDetails?.assignment?.username;
500+
501+
const reviewStats = addReviewStats(job.localPath, job.mrNumber, durationMs, stdout, assignedBy);
502+
logger.info({ reviewStats }, 'Stats de review enregistrées');
503+
} catch (statsError) {
504+
logger.warn({ error: statsError }, 'Erreur lors de l\'enregistrement des stats');
505+
}
505506
}
506507
// Log stdout preview for debugging
507508
if (stdout.length > 0) {

src/interface-adapters/views/dashboard/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,7 @@ <h1>Reviewflow</h1>
844844
if (mr.totalBlocking > 0) statsBadges.push(`<span class="stat-badge blocking"><i data-lucide="octagon-alert"></i> ${mr.totalBlocking}</span>`);
845845
if (mr.totalWarnings > 0) statsBadges.push(`<span class="stat-badge warning"><i data-lucide="alert-triangle"></i> ${mr.totalWarnings}</span>`);
846846
if (mr.totalFollowups > 0) statsBadges.push(`<span class="stat-badge followup"><i data-lucide="refresh-cw"></i> ${mr.totalFollowups}</span>`);
847-
if (mr.averageScore !== null) statsBadges.push(`<span class="stat-badge score"><i data-lucide="star"></i> ${mr.averageScore.toFixed(1)}/10</span>`);
847+
if (mr.latestScore !== null) statsBadges.push(`<span class="stat-badge score"><i data-lucide="star"></i> ${mr.latestScore.toFixed(1)}/10</span>`);
848848

849849
const durationFormatted = mr.totalDurationMs ? formatDuration(null, null, mr.totalDurationMs) : '';
850850

src/tests/factories/trackedMr.factory.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export class TrackedMrFactory {
2828
totalWarnings: 0,
2929
totalSuggestions: 0,
3030
totalDurationMs: 0,
31-
averageScore: null,
3231
latestScore: null,
3332
autoFollowup: true,
3433
...overrides,

src/tests/units/usecases/tracking/recordReviewCompletion.usecase.test.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ describe('RecordReviewCompletionUseCase', () => {
3131
expect(result?.totalSuggestions).toBe(1);
3232
expect(result?.totalDurationMs).toBe(60000);
3333
expect(result?.latestScore).toBe(8);
34-
expect(result?.averageScore).toBe(8);
3534
});
3635

3736
it('should transition to pending-fix when blocking issues exist', () => {
@@ -80,12 +79,31 @@ describe('RecordReviewCompletionUseCase', () => {
8079
expect(result?.totalThreads).toBe(3);
8180
});
8281

83-
it('should compute average score across reviews', () => {
82+
it('should record followup type separately', () => {
83+
const gateway = new InMemoryReviewRequestTrackingGateway();
84+
const mr = TrackedMrFactory.create({ id: 'mr-1' });
85+
gateway.create('/project', mr);
86+
const useCase = new RecordReviewCompletionUseCase(gateway);
87+
88+
const result = useCase.execute({
89+
projectPath: '/project',
90+
mrId: 'mr-1',
91+
reviewData: { ...reviewData, type: 'followup' },
92+
});
93+
94+
expect(result?.totalFollowups).toBe(1);
95+
expect(result?.totalReviews).toBe(0);
96+
});
97+
98+
it('should set latestScore to the most recent review score', () => {
8499
const gateway = new InMemoryReviewRequestTrackingGateway();
85100
const mr = TrackedMrFactory.create({
86101
id: 'mr-1',
87-
reviews: [{ type: 'review', timestamp: '2024-01-01T00:00:00Z', durationMs: 1000, score: 6, blocking: 0, warnings: 0, suggestions: 0, threadsClosed: 0, threadsOpened: 0 }],
102+
reviews: [
103+
{ type: 'review', timestamp: '2024-01-01T00:00:00Z', durationMs: 1000, score: 6, blocking: 0, warnings: 0, suggestions: 0, threadsClosed: 0, threadsOpened: 0 },
104+
],
88105
totalReviews: 1,
106+
latestScore: 6,
89107
});
90108
gateway.create('/project', mr);
91109
const useCase = new RecordReviewCompletionUseCase(gateway);
@@ -96,24 +114,39 @@ describe('RecordReviewCompletionUseCase', () => {
96114
reviewData: { ...reviewData, score: 10 },
97115
});
98116

99-
expect(result?.averageScore).toBe(8);
100117
expect(result?.latestScore).toBe(10);
101118
});
102119

103-
it('should record followup type separately', () => {
120+
it('should update latestScore from followup', () => {
104121
const gateway = new InMemoryReviewRequestTrackingGateway();
105-
const mr = TrackedMrFactory.create({ id: 'mr-1' });
122+
const mr = TrackedMrFactory.create({
123+
id: 'mr-1',
124+
reviews: [
125+
{ type: 'review', timestamp: '2024-01-01T00:00:00Z', durationMs: 1000, score: 5, blocking: 1, warnings: 0, suggestions: 0, threadsClosed: 0, threadsOpened: 1 },
126+
],
127+
totalReviews: 1,
128+
latestScore: 5,
129+
openThreads: 1,
130+
totalThreads: 1,
131+
});
106132
gateway.create('/project', mr);
107133
const useCase = new RecordReviewCompletionUseCase(gateway);
108134

109135
const result = useCase.execute({
110136
projectPath: '/project',
111137
mrId: 'mr-1',
112-
reviewData: { ...reviewData, type: 'followup' },
138+
reviewData: {
139+
type: 'followup',
140+
durationMs: 30000,
141+
score: 8,
142+
blocking: 0,
143+
warnings: 0,
144+
suggestions: 0,
145+
threadsClosed: 1,
146+
},
113147
});
114148

115-
expect(result?.totalFollowups).toBe(1);
116-
expect(result?.totalReviews).toBe(0);
149+
expect(result?.latestScore).toBe(8);
117150
});
118151

119152
it('should return null for unknown MR', () => {

src/usecases/tracking/recordReviewCompletion.usecase.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,12 @@ export class RecordReviewCompletionUseCase implements UseCase<RecordReviewComple
5656
latestScore = reviewData.score;
5757
}
5858

59-
const reviewsWithScore = afterEvent.reviews.filter((r) => r.score !== null);
60-
let averageScore: number | null = null;
61-
if (reviewsWithScore.length > 0) {
62-
averageScore = reviewsWithScore.reduce((sum, r) => sum + (r.score ?? 0), 0) / reviewsWithScore.length;
63-
}
64-
6559
const hasBlockingIssues = reviewData.blocking > 0 || openThreads > 0;
6660

6761
this.trackingGateway.update(projectPath, mrId, {
6862
openThreads,
6963
totalThreads,
7064
latestScore,
71-
averageScore,
7265
state: hasBlockingIssues ? 'pending-fix' : 'pending-approval',
7366
});
7467

src/usecases/tracking/trackAssignment.usecase.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ export class TrackAssignmentUseCase implements UseCase<TrackAssignmentInput, Tra
9393
totalWarnings: 0,
9494
totalSuggestions: 0,
9595
totalDurationMs: 0,
96-
averageScore: null,
9796
latestScore: null,
9897
autoFollowup: true,
9998
};

0 commit comments

Comments
 (0)