fix: address review feedback for quality score display

- Extract shared critic response parser into helper, removing duplicate
  Zod schemas and eliminating stale score overwrite from progress log
- Use CDK overlay for badge tooltip to prevent table overflow clipping
- Add keyboard/screen-reader accessibility to badge tooltip
- Replace toFixed() template calls with QualityScoreLabelPipe
- Reduce agent timeout from 10min to 5min
- Fix Prisma schema field alignment
- Make second critic validation score-only (ignore revision suggestions)

Signed-off-by: Asitha de Silva <asithade@gmail.com>

Author: asithade

apps/lfx-changelog/prisma/schema.prisma

@@ -149,9 +149,9 @@ model ChangelogEntry {
   title       String
   description String
   version     String?
-  source       ChangelogSource   @default(manual) @map("source")
-  status       ChangelogStatus   @default(draft)
-  qualityScore Json?             @map("quality_score")
+  source      ChangelogSource   @default(manual) @map("source")
+  status      ChangelogStatus   @default(draft)
+  qualityScore Json?            @map("quality_score")
   publishedAt DateTime?         @map("published_at")
   createdBy   String            @map("created_by")
   createdAt   DateTime          @default(now()) @map("created_at")

apps/lfx-changelog/src/app/modules/admin/changelog-list/changelog-list.component.html

@@ -81,7 +81,7 @@ <h1 data-testid="changelog-list-heading" class="font-display text-text-primary t
 
       <ng-template lfxColumn="Score" let-entry>
         @if (entry.qualityScore; as qs) {
-          <lfx-badge [label]="qs.overall.toFixed(1) + '/5'" [color]="qs.overall | qualityScoreColor" size="sm" [hasTooltip]="true">
+          <lfx-badge [label]="qs.overall | qualityScoreLabel" [color]="qs.overall | qualityScoreColor" size="sm" [hasTooltip]="true">
             <div tooltip class="w-40">
               <p class="text-text-secondary mb-1.5 text-[10px] font-semibold tracking-wider uppercase">Quality Breakdown</p>
               <div class="flex items-center justify-between py-0.5 text-xs">
@@ -159,7 +159,7 @@ <h1 data-testid="changelog-list-heading" class="font-display text-text-primary t
               <div class="flex flex-wrap items-center gap-2">
                 <lfx-status-badge [status]="entry.status" />
                 @if (entry.qualityScore; as qs) {
-                  <lfx-badge [label]="qs.overall.toFixed(1) + '/5'" [color]="qs.overall | qualityScoreColor" size="sm" />
+                  <lfx-badge [label]="qs.overall | qualityScoreLabel" [color]="qs.overall | qualityScoreColor" size="sm" />
                 } @else {
                   <span class="text-text-muted text-xs">N/A</span>
                 }

apps/lfx-changelog/src/app/modules/admin/changelog-list/changelog-list.component.ts

@@ -26,6 +26,7 @@ import { DateFormatPipe } from '@shared/pipes/date-format.pipe';
 import { MapGetPipe } from '@shared/pipes/map-get.pipe';
 import { ProductNamePipe } from '@shared/pipes/product-name.pipe';
 import { QualityScoreColorPipe } from '@shared/pipes/quality-score-color.pipe';
+import { QualityScoreLabelPipe } from '@shared/pipes/quality-score-label.pipe';
 import { BehaviorSubject, catchError, combineLatest, map, of, startWith, switchMap, take, tap } from 'rxjs';
 
 import type { ChangelogEntryWithRelations, PaginatedResponse, Product } from '@lfx-changelog/shared';
@@ -49,6 +50,7 @@ import type { DropdownMenuItem, SelectOption } from '@shared/interfaces/form.int
     MapGetPipe,
     ProductNamePipe,
     QualityScoreColorPipe,
+    QualityScoreLabelPipe,
   ],
   templateUrl: './changelog-list.component.html',
   styleUrl: './changelog-list.component.css',

apps/lfx-changelog/src/app/shared/components/badge/badge.component.html

@@ -2,16 +2,26 @@
 <!-- SPDX-License-Identifier: MIT -->
 
 <span
-  class="group relative inline-flex items-center gap-1 rounded-full font-medium"
+  class="inline-flex items-center gap-1 rounded-full font-medium"
   [class]="size() === 'sm' ? 'px-2 py-0.5 text-xs' : 'px-2.5 py-1 text-xs'"
   [class.cursor-default]="hasTooltip()"
+  [attr.tabindex]="hasTooltip() ? '0' : null"
+  cdkOverlayOrigin
+  #trigger="cdkOverlayOrigin"
+  (mouseenter)="showTooltip()"
+  (mouseleave)="hideTooltip()"
+  (focus)="showTooltip()"
+  (blur)="hideTooltip()"
   [style]="bgStyle()">
   {{ label() }}
-
-  @if (hasTooltip()) {
-    <div
-      class="bg-surface border-border text-text-primary pointer-events-none absolute bottom-full left-1/2 z-50 mb-2 hidden -translate-x-1/2 rounded-lg border p-2.5 shadow-lg group-hover:block">
-      <ng-content select="[tooltip]" />
-    </div>
-  }
 </span>
+
+<ng-template
+  cdkConnectedOverlay
+  [cdkConnectedOverlayOrigin]="trigger"
+  [cdkConnectedOverlayOpen]="hasTooltip() && isTooltipOpen()"
+  [cdkConnectedOverlayPositions]="positions">
+  <div role="tooltip" class="bg-surface border-border text-text-primary pointer-events-none rounded-lg border p-2.5 shadow-lg">
+    <ng-content select="[tooltip]" />
+  </div>
+</ng-template>

apps/lfx-changelog/src/app/shared/components/badge/badge.component.ts

@@ -1,10 +1,12 @@
 // Copyright The Linux Foundation and each contributor to LFX.
 // SPDX-License-Identifier: MIT
 
-import { Component, computed, input } from '@angular/core';
+import { CdkConnectedOverlay, CdkOverlayOrigin, ConnectedPosition } from '@angular/cdk/overlay';
+import { Component, computed, input, signal } from '@angular/core';
 
 @Component({
   selector: 'lfx-badge',
+  imports: [CdkOverlayOrigin, CdkConnectedOverlay],
   templateUrl: './badge.component.html',
   styleUrl: './badge.component.css',
 })
@@ -14,8 +16,23 @@ export class BadgeComponent {
   public readonly size = input<'sm' | 'md'>('md');
   public readonly hasTooltip = input(false);
 
+  protected readonly isTooltipOpen = signal(false);
+
+  protected readonly positions: ConnectedPosition[] = [
+    { originX: 'center', originY: 'bottom', overlayX: 'center', overlayY: 'top', offsetY: 4 },
+    { originX: 'center', originY: 'top', overlayX: 'center', overlayY: 'bottom', offsetY: -4 },
+  ];
+
   protected readonly bgStyle = computed(() => {
     const hex = this.color();
     return `background-color: ${hex}1a; color: ${hex};`;
   });
+
+  protected showTooltip(): void {
+    this.isTooltipOpen.set(true);
+  }
+
+  protected hideTooltip(): void {
+    this.isTooltipOpen.set(false);
+  }
 }

apps/lfx-changelog/src/app/shared/pipes/quality-score-label.pipe.ts

@@ -0,0 +1,14 @@
+// Copyright The Linux Foundation and each contributor to LFX.
+// SPDX-License-Identifier: MIT
+
+import { Pipe, PipeTransform } from '@angular/core';
+
+@Pipe({
+  name: 'qualityScoreLabel',
+  standalone: true,
+})
+export class QualityScoreLabelPipe implements PipeTransform {
+  public transform(overall: number): string {
+    return `${overall.toFixed(1)}/5`;
+  }
+}

apps/lfx-changelog/src/server/constants/agent.constants.ts

@@ -4,7 +4,7 @@
 export const AGENT_CONFIG = {
   MAX_TURNS: 15,
   MODEL: 'claude-sonnet-4-6',
-  TIMEOUT_MS: 600_000, // 10 minutes
+  TIMEOUT_MS: 300_000, // 5 minutes
 } as const;
 
 /** Changelog MCP tools the agent is allowed to call. */
@@ -62,7 +62,7 @@ Your job is to produce a polished, user-focused changelog entry from raw GitHub
    - Description has at least 2 grouped headings for non-trivial updates (5+ changes)
    - If validation fails, revise and re-validate (up to 2 retries)
 5. **REVIEW** — call \`validate_changelog_draft\` to get a quality score from the critic.
-   - If the critic suggests revisions, apply them using \`update_changelog_draft\`, then call \`validate_changelog_draft\` exactly once more for a final score. You MUST NOT call \`validate_changelog_draft\` more than 2 times total per run.
+   - If the critic suggests revisions, apply them using \`update_changelog_draft\`, then call \`validate_changelog_draft\` exactly once more to record the final score. **Ignore any revision suggestions from this second call** — it is strictly for scoring. You MUST NOT call \`validate_changelog_draft\` more than 2 times total per run.
    - Skip the critic if the activity is trivial (fewer than 3 commits/PRs total).
 6. **SAVE** via \`create_changelog_draft\` (new entry) or \`update_changelog_draft\` (existing draft ID provided in context).
 

apps/lfx-changelog/src/server/helpers/critic-response.helper.ts

@@ -0,0 +1,36 @@
+// Copyright The Linux Foundation and each contributor to LFX.
+// SPDX-License-Identifier: MIT
+
+import { z } from 'zod';
+
+const CriticResponseSchema = z.object({
+  scores: z.object({
+    accuracy: z.number().min(1).max(5),
+    clarity: z.number().min(1).max(5),
+    tone: z.number().min(1).max(5),
+    completeness: z.number().min(1).max(5),
+  }),
+  overall: z.number().min(1).max(5),
+});
+
+export type QualityScore = { accuracy: number; clarity: number; tone: number; completeness: number; overall: number };
+
+/**
+ * Extracts quality scores from a critic response string.
+ * Handles both raw JSON and JSON wrapped in markdown code fences.
+ * Returns null if the response cannot be parsed or doesn't match the schema.
+ */
+export function parseCriticScores(text: string): QualityScore | null {
+  try {
+    const jsonMatch = text.match(/```(?:json)?\s*([\s\S]*?)```/) || [null, text];
+    const raw = JSON.parse(jsonMatch[1]!.trim());
+    const parseResult = CriticResponseSchema.safeParse(raw);
+    if (parseResult.success) {
+      const { scores, overall } = parseResult.data;
+      return { ...scores, overall };
+    }
+    return null;
+  } catch {
+    return null;
+  }
+}

apps/lfx-changelog/src/server/services/changelog-agent.service.ts

@@ -16,6 +16,7 @@ import {
 import { AgentServiceError } from '../errors';
 import { buildActivityContext } from '../helpers/activity-context.helper';
 import { extractAtlassianReferences, formatAtlassianHints } from '../helpers/atlassian-reference.helper';
+import { parseCriticScores } from '../helpers/critic-response.helper';
 import { serverLogger } from '../server-logger';
 import { agentJobEmitter } from './agent-job-emitter.service';
 import { AgentMemoryService } from './agent-memory.service';
@@ -473,11 +474,6 @@ export class ChangelogAgentService {
               serverLogger.info({ jobId, productId, durationMs, numTurns: message.num_turns }, 'Agent job completed successfully');
               agentJobEmitter.emit(jobId, { type: 'status', data: { status: 'completed' } });
 
-              // Record quality scores from critic (fire-and-forget)
-              this.recordScoresFromProgressLog(jobId, productId, updatedJob.changelogEntry?.id ?? null, progressLog).catch((err) =>
-                serverLogger.warn({ err, jobId }, 'Failed to record quality scores')
-              );
-
               // Notify configured users via Slack DM — release-triggered jobs only (fire-and-forget)
               if (updatedJob.trigger === 'webhook_release' && updatedJob.changelogEntry && updatedJob.product) {
                 const entry = { id: updatedJob.changelogEntry.id, title: updatedJob.changelogEntry.title };
@@ -847,30 +843,12 @@ export class ChangelogAgentService {
             }
           }
 
-          // Try to extract and record quality scores from the critic response
-          try {
-            // Handle pure JSON or JSON wrapped in markdown code fences
-            const jsonMatch = criticResponse.match(/```(?:json)?\s*([\s\S]*?)```/) || [null, criticResponse];
-            const raw = JSON.parse(jsonMatch[1]!.trim());
-            const CriticResponseSchema = z.object({
-              scores: z.object({
-                accuracy: z.number().min(1).max(5),
-                clarity: z.number().min(1).max(5),
-                tone: z.number().min(1).max(5),
-                completeness: z.number().min(1).max(5),
-              }),
-              overall: z.number().min(1).max(5),
-            });
-            const parseResult = CriticResponseSchema.safeParse(raw);
-            if (parseResult.success) {
-              const { scores, overall } = parseResult.data;
-              const qualityScore = { ...scores, overall };
-              await agentMemoryService.recordQualityScores(jobId, productId, qualityScore);
-              await prisma.changelogEntry.update({ where: { id: args.draftId }, data: { qualityScore } });
-              serverLogger.info({ jobId, productId, draftId: args.draftId, overall }, 'Recorded quality scores from critic');
-            }
-          } catch {
-            serverLogger.debug({ jobId, productId }, 'Could not extract quality scores from critic response');
+          // Extract and record quality scores from the critic response
+          const qualityScore = parseCriticScores(criticResponse);
+          if (qualityScore) {
+            await agentMemoryService.recordQualityScores(jobId, productId, qualityScore);
+            await prisma.changelogEntry.update({ where: { id: args.draftId }, data: { qualityScore } });
+            serverLogger.info({ jobId, productId, draftId: args.draftId, overall: qualityScore.overall }, 'Recorded quality scores from critic');
           }
 
           return { content: [{ type: 'text' as const, text: criticResponse }] };
@@ -887,52 +865,4 @@ export class ChangelogAgentService {
       tools: [searchPastChangelogs, createChangelogDraft, updateChangelogDraft, getLatestVersion, validateChangelogDraft],
     });
   }
-
-  /**
-   * Walks the progress log to find critic scores from validate_changelog_draft.
-   * Looks for a text entry after the tool call that contains valid JSON scores.
-   */
-  private async recordScoresFromProgressLog(jobId: string, productId: string, changelogId: string | null, progressLog: ProgressLogEntry[]): Promise<void> {
-    const CriticResponseSchema = z.object({
-      scores: z.object({
-        accuracy: z.number().min(1).max(5),
-        clarity: z.number().min(1).max(5),
-        tone: z.number().min(1).max(5),
-        completeness: z.number().min(1).max(5),
-      }),
-      overall: z.number().min(1).max(5),
-    });
-
-    for (let i = 0; i < progressLog.length; i++) {
-      const entry = progressLog[i];
-      if (entry.type !== 'tool_call' || entry.tool !== 'validate_changelog_draft') continue;
-
-      // The critic response appears as a subsequent text entry
-      for (let j = i + 1; j < progressLog.length; j++) {
-        const nextEntry = progressLog[j];
-        if (nextEntry.type === 'text' && nextEntry.summary) {
-          try {
-            const raw = JSON.parse(nextEntry.summary);
-            const parseResult = CriticResponseSchema.safeParse(raw);
-            if (parseResult.success) {
-              const { scores, overall } = parseResult.data;
-              const qualityScore = { ...scores, overall };
-              await this.agentMemoryService.recordQualityScores(jobId, productId, qualityScore);
-
-              // Denormalize onto the changelog entry for admin UI display
-              if (changelogId) {
-                const prisma = getPrismaClient();
-                await prisma.changelogEntry.update({ where: { id: changelogId }, data: { qualityScore } });
-              }
-              return;
-            }
-          } catch {
-            // Not valid JSON — continue looking
-          }
-        }
-        // Stop searching if we hit another tool call
-        if (nextEntry.type === 'tool_call') break;
-      }
-    }
-  }
 }