Reviewing files that changed from the base of the PR and between 12e420b and 01dde0e.

• frontend/app/src/entities/diff/api/get-diff-tree-summary-from-api.ts (1 hunks)
• frontend/app/src/entities/diff/domain/get-diff-summary.query.ts (1 hunks)
• frontend/app/src/entities/diff/domain/get-diff-summary.ts (1 hunks)
• frontend/app/src/entities/proposed-changes/ui/diff-summary.tsx (3 hunks)

• GitHub Check: E2E-testing-version-upgrade / From 1.3.0
• GitHub Check: E2E-testing-playwright
• GitHub Check: Cloudflare Pages

frontend/app/src/entities/diff/domain/get-diff-summary.query.ts (2)

1-5: LGTM! Clean imports aligned with TanStack Query v5.

The imports correctly use the @tanstack/react-query package and properly structure the dependencies across API, domain, and shared layers.

---

16-18: LGTM! Proper TanStack Query v5 hook pattern.

The hook correctly merges query options with optional config overrides using the spread operator, following TanStack Query best practices.

frontend/app/src/entities/proposed-changes/ui/diff-summary.tsx (4)

7-7: LGTM! Clean migration to new data-fetching architecture.

The imports correctly reference the new useGetDiffSummary hook and supporting types, replacing the previous direct GraphQL query approach.

Also applies to: 10-11

---

43-61: LGTM! Excellent state handling with TanStack Query.

The component correctly uses isPending (TanStack Query v5 property), provides explicit error handling with user-friendly messages, and includes a defensive null check before accessing data properties.

---

64-86: LGTM! Clean render logic with proper data access.

The component correctly accesses diff summary properties directly from the data object, protected by the null check above. The Row wrapper provides consistent layout.

---

91-95: LGTM! Consistent skeleton layout.

The skeleton correctly uses the same Row wrapper and renders 4 placeholder badges to match the actual rendered state, ensuring a smooth loading experience.

frontend/app/src/entities/diff/domain/get-diff-summary.ts (2)

1-14: LGTM! Clear and well-structured type definitions.

The types provide a clean contract for the domain layer, with proper nullable return type and clear parameter/response shapes.

---

16-26: Review type assertion for potential runtime safety issues.

The implementation has two minor concerns:

1. Line 25: The type assertion as GetDiffSummaryResponse bypasses type safety. If the GraphQL response shape diverges from the type definition (e.g., missing fields or different types), this could cause runtime errors. Consider adding runtime validation or using a type guard.

2. Line 25: The null coalescing ?? null is redundant since if data.DiffTreeSummary is falsy, it will already return null or the falsy value. The ?? null doesn't change behavior.

Consider validating the response shape at runtime or documenting the assumption that the GraphQL generated types guarantee the shape. You could use a library like zod or add a simple type guard:

function isValidDiffSummary(data: any): data is GetDiffSummaryResponse {
  return (
    typeof data?.num_added === 'number' &&
    typeof data?.num_updated === 'number' &&
    typeof data?.num_removed === 'number' &&
    typeof data?.num_conflicts === 'number'
  );
}

export const getDiffSummary: GetDiffSummary = async ({ branchName }) => {
  const { data, errors } = await getDiffTreeSummaryFromApi({
    branch: branchName,
  });

  if (errors) {
    throw new Error(errors.map((e) => e.message).join("; "));
  }

  const summary = data.DiffTreeSummary;
  if (!summary) return null;
  
  if (!isValidDiffSummary(summary)) {
    throw new Error("Invalid diff summary response shape");
  }
  
  return summary;
};

frontend/app/src/entities/diff/api/get-diff-tree-summary-from-api.ts (3)

1-7: LGTM! Proper GraphQL setup with generated types.

The imports correctly reference Apollo Client utilities and generated TypeScript types from GraphQL codegen.

---

9-18: LGTM! Well-structured GraphQL query.

The query correctly defines the operation with appropriate parameters and selects all necessary fields for the diff summary.

---

22-30: Verify intentional error suppression.

The processErrorMessage: () => {} in the context suppresses default GraphQL error processing, shifting error handling responsibility to the domain layer (where getDiffSummary throws on errors). This is a valid pattern, but please confirm this is intentional.

If this is intentional, consider adding a comment explaining why error processing is suppressed:

context: {
  // Suppress default error processing; errors are handled in the domain layer
  processErrorMessage: () => {},
},