Skip to content

Conversation

@own-boldsbrain
Copy link
Member

Summary

  • add EnrichmentAuditCard component with latency histogram, cache/fallback badges, log link and export options
  • support latency histogram aggregation helper
  • test latency histogram aggregation

Testing

  • pnpm lint packages/ui-cards/EnrichmentAuditCard.tsx
  • pnpm exec vitest packages/ui-cards/EnrichmentAuditCard.test.ts packages/ui-cards/RiskScoreCard.test.ts (fails: Cannot find package '@/tests/prompts/utils' imported from '/workspace/ai-ysh/lib/ai/models.test.ts')

https://chatgpt.com/codex/tasks/task_e_68ba522a7a4883328049bf7968b0021f

Copilot AI review requested due to automatic review settings September 5, 2025 13:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new React component for displaying enrichment audit data with visualization capabilities. The component provides a comprehensive view of data enrichment performance metrics including latency histograms, cache statistics, and export functionality.

Key Changes:

  • Added EnrichmentAuditCard component with latency histogram visualization, cache/fallback badges, and export options
  • Implemented buildLatencyHistogram helper function for aggregating latency data into bins
  • Added comprehensive test coverage for the histogram aggregation logic

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/ui-cards/EnrichmentAuditCard.tsx New React component with Zod schema validation, histogram visualization, and PNG/JSON export functionality
packages/ui-cards/EnrichmentAuditCard.test.ts Unit tests for the latency histogram aggregation helper function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +46 to +51
const blob = new Blob([json], { type: 'application/json' });
const url = URL.createObjectURL(blob);
const link = document.createElement('a');
link.download = 'enrichment-audit.json';
link.href = url;
link.click();
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak: URL.createObjectURL creates a blob URL that should be revoked after use to free memory. Add URL.revokeObjectURL(url) after link.click().

Suggested change
const blob = new Blob([json], { type: 'application/json' });
const url = URL.createObjectURL(blob);
const link = document.createElement('a');
link.download = 'enrichment-audit.json';
link.href = url;
link.click();
link.click();
URL.revokeObjectURL(url);

Copilot uses AI. Check for mistakes.
>
{histogram.map((count, i) => {
const height = (count / maxCount) * 100;
const rangeLabel = `${i * binSize}-${i * binSize + binSize}`;
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The range calculation i * binSize + binSize can be simplified to (i + 1) * binSize for better readability.

Suggested change
const rangeLabel = `${i * binSize}-${i * binSize + binSize}`;
const rangeLabel = `${i * binSize}-${(i + 1) * binSize}`;

Copilot uses AI. Check for mistakes.
export function buildLatencyHistogram(
latencies: number[],
binSize = 50,
): number[] {
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When latencies array is empty, Math.max(0, ...latencies) will return 0, but Math.floor(0 / binSize) + 1 will create 1 bin instead of 0 bins. This creates inconsistency with the test expectation that expects [0] for empty input.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants