What happened
On PR #4395, the review agent ran 6 times (run 1, run 2, run 3, run 4, run 5, run 6) and approved every time with only low-severity code-pattern findings. Two human reviewers then caught substantive issues the agent missed:
- Eswaraiahsapram (review) identified that empty/error scalar aggregates — where all entities fail calculation (
total === 0 && calculationErrorCount > 0) — rendered as a green 0 value instead of defaulting to grey. This is a functional correctness bug in the UI state handling.
- imykhno (comment) identified that scalar type labels (Min, Max, Sum, Count, Average) were placed under the
metric. i18n namespace instead of aggregation., violating the workspace's implicit namespace convention for aggregation-specific terminology.
The current workspaces/scorecard/AGENTS.md is entirely backend-focused: it documents metric providers, threshold resolution, metric ID naming, and config schema. It has zero frontend guidance — no component conventions, no empty/error state patterns, and no i18n key naming conventions.
What could go better
The review agent had no documented frontend conventions to reference when reviewing UI component code. It focused on what it could infer from code patterns alone — type definition consistency, regex edge cases, JSX syntax. The two issues humans caught both required domain knowledge that exists implicitly in the team but is not captured in AGENTS.md:
- Empty/error state rendering: The convention that cards with no successful samples should render as grey (not success-green) is a UX principle the team enforces but hasn't documented.
- i18n key namespaces: The convention that aggregation-specific labels belong under
aggregation.* (not metric.*) reflects a deliberate namespace structure in the translation files.
Confidence is high that documenting these conventions would help both the review agent and code agent (which uses Cursor in this workflow). The agent demonstrably reads and applies AGENTS.md guidance — its threshold-resolution review guidance already shows this. The gap is simply that frontend conventions are absent from the file.
Proposed change
Add a ## Frontend Component Conventions section to workspaces/scorecard/AGENTS.md covering:
-
i18n translation key namespaces: Document that translation keys are organized by domain — metric.<id> for individual metric display names, aggregation.<type> for aggregation type labels (min, max, sum, count, average), and any other established prefixes. Reference ref.ts as the canonical namespace map.
-
Empty and error state rendering: Document that when a scalar aggregation has no successful samples (total === 0 and calculationErrorCount > 0), the card must render in a neutral/grey state — not apply threshold coloring that would imply a valid result. More generally, state the principle: do not render success/failure coloring when the underlying data is absent or entirely errored.
-
AggregatedMetricCard dispatch pattern: Document that AggregatedMetricCard uses result-shape-based discrimination (numeric value field vs. values[] array) rather than type-name matching to select the card variant. This was a deliberate architectural choice in this PR that the review agent noted but flagged as scope creep.
-
Frontend/backend responsibility boundary for threshold evaluation: Note that threshold evaluation is the backend's responsibility. The frontend mirrors the logic only for coloring display purposes and should defer to backend-provided values when available (follow-up tracked in RHIDP-16510).
Validation criteria
On the next 3 scorecard workspace PRs that modify frontend components or translation files, the review agent should (a) reference the documented conventions when reviewing i18n key additions and (b) flag violations of the empty-state rendering pattern if present. If the review agent's findings on those PRs include at least one domain-convention-based observation (rather than only code-pattern findings), the improvement is validated.
Generated by retro agent from #4395
What happened
On PR #4395, the review agent ran 6 times (run 1, run 2, run 3, run 4, run 5, run 6) and approved every time with only low-severity code-pattern findings. Two human reviewers then caught substantive issues the agent missed:
total === 0 && calculationErrorCount > 0) — rendered as a green 0 value instead of defaulting to grey. This is a functional correctness bug in the UI state handling.metric.i18n namespace instead ofaggregation., violating the workspace's implicit namespace convention for aggregation-specific terminology.The current
workspaces/scorecard/AGENTS.mdis entirely backend-focused: it documents metric providers, threshold resolution, metric ID naming, and config schema. It has zero frontend guidance — no component conventions, no empty/error state patterns, and no i18n key naming conventions.What could go better
The review agent had no documented frontend conventions to reference when reviewing UI component code. It focused on what it could infer from code patterns alone — type definition consistency, regex edge cases, JSX syntax. The two issues humans caught both required domain knowledge that exists implicitly in the team but is not captured in AGENTS.md:
aggregation.*(notmetric.*) reflects a deliberate namespace structure in the translation files.Confidence is high that documenting these conventions would help both the review agent and code agent (which uses Cursor in this workflow). The agent demonstrably reads and applies AGENTS.md guidance — its threshold-resolution review guidance already shows this. The gap is simply that frontend conventions are absent from the file.
Proposed change
Add a
## Frontend Component Conventionssection toworkspaces/scorecard/AGENTS.mdcovering:i18n translation key namespaces: Document that translation keys are organized by domain —
metric.<id>for individual metric display names,aggregation.<type>for aggregation type labels (min, max, sum, count, average), and any other established prefixes. Referenceref.tsas the canonical namespace map.Empty and error state rendering: Document that when a scalar aggregation has no successful samples (
total === 0andcalculationErrorCount > 0), the card must render in a neutral/grey state — not apply threshold coloring that would imply a valid result. More generally, state the principle: do not render success/failure coloring when the underlying data is absent or entirely errored.AggregatedMetricCard dispatch pattern: Document that
AggregatedMetricCarduses result-shape-based discrimination (numericvaluefield vs.values[]array) rather than type-name matching to select the card variant. This was a deliberate architectural choice in this PR that the review agent noted but flagged as scope creep.Frontend/backend responsibility boundary for threshold evaluation: Note that threshold evaluation is the backend's responsibility. The frontend mirrors the logic only for coloring display purposes and should defer to backend-provided values when available (follow-up tracked in RHIDP-16510).
Validation criteria
On the next 3 scorecard workspace PRs that modify frontend components or translation files, the review agent should (a) reference the documented conventions when reviewing i18n key additions and (b) flag violations of the empty-state rendering pattern if present. If the review agent's findings on those PRs include at least one domain-convention-based observation (rather than only code-pattern findings), the improvement is validated.
Generated by retro agent from #4395