fix: Resolve lint errors from pre-push checks

[amikofalvy]

Summary

Fixed multiple lint errors that were causing pre-push checks to fail.

Changes

• Fixed template literal usage in format-date.ts
• Removed unused imports and state variables in inline-event.tsx
• Removed unused useAgentStore import in sidepane.tsx
• Wrapped async functions in useCallback with proper dependencies in multiple files
• Fixed non-null assertions by adding conditional checks
• Cleaned up React hook dependency arrays

Lint Errors Fixed

1. String concatenation → template literals
2. Unused imports (ChevronRight, cn, useAgentStore)
3. Unused state variables
4. Missing useCallback wrappers
5. Incorrect React hook dependencies
6. Non-null assertion operators

All original pre-push lint errors have been resolved.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: b17fddaea952d8038320e468ad22106803b9a96a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
agents-manage-api	Ready	Preview	Comment	Nov 21, 2025 0:13am
agents-manage-ui	Error			Nov 21, 2025 0:13am
agents-run-api	Ready	Preview	Comment	Nov 21, 2025 0:13am

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Skipped			Nov 21, 2025 0:13am

[claude[bot]]

Claude finished @amikofalvy's task —— View job

---

Code Review: Lint Error Fixes

I've thoroughly analyzed this lint fix PR and have several observations about the code quality and architectural considerations.

Progress:

• [x] Read repository documentation (AGENTS.md, CLAUDE.md)
• [x] Analyze changed files to understand lint fixes
• [x] Evaluate code quality and architectural considerations
• [x] Check for consistency and best practices
• [x] Provide comprehensive review feedback

Summary

This PR successfully addresses multiple lint errors with generally good fixes, but there are several areas that warrant deeper consideration from both a code quality and system design perspective.

Positive Changes ✅

String Concatenation → Template Literal: The change in format-date.ts:17 from dateString.replace(' ', 'T') + 'Z' to `${dateString.replace(' ', 'T')}Z` follows modern ES6 practices and improves readability.

Unused Import Cleanup: Systematic removal of unused imports (ChevronRight, cn, useAgentStore) and state variables (isExpanded, getExpandedContent) demonstrates good housekeeping.

Areas for Consideration 🤔

1. Missing useCallback Dependencies - Critical

Several files show a concerning pattern where useCallback was added but with incomplete dependency arrays:

agents-manage-ui/src/components/evaluation-jobs/evaluation-job-form-dialog.tsx:78-89

const loadData = useCallback(async () => {
  // ... uses tenantId, projectId
}, [tenantId, projectId]);

This is correct, but then in the useEffect at line 92:

useEffect(() => {
  if (isOpen) {
    form.reset(formatFormData(initialData));
    loadData();
  }
}, [isOpen, initialData, form, loadData]);

Issue: The useEffect includes form and loadData but formatFormData function (called within) might have hidden dependencies. Since formatFormData uses initialData, this looks correct, but we should verify that form.reset doesn't cause unnecessary re-renders.

2. Potential Performance Anti-pattern

In multiple files, you're wrapping async functions in useCallback that are only called from useEffect. Consider if these functions truly need memoization:

suite-config-details-popover.tsx:38-104

const loadSuiteConfigDetails = useCallback(async () => {
  // Heavy async operation
}, [suiteConfigId, tenantId, projectId]);

useEffect(() => {
  if (isOpen && !suiteConfig) {
    loadSuiteConfigDetails();
  }
}, [isOpen, suiteConfigId, tenantId, projectId, suiteConfig, loadSuiteConfigDetails]);

Concern: This pattern may be over-optimization. The async function is only used in one useEffect and the dependencies make it recreate frequently anyway. The memoization benefit is minimal.

3. Incomplete Refactoring in inline-event.tsx

The file still contains large blocks of commented code (lines 37-71). This suggests:

• Code Debt: Dead code should be removed entirely rather than commented out
• Incomplete Feature: If this represents a feature being developed, it should be in a feature branch
• Missing Context: The commented code includes expansion functionality that might be needed later

Recommendation: Either remove the dead code entirely or create a ticket to complete the feature implementation.

4. Dependency Array Inconsistencies

Looking at sidepane.tsx:172-186, you removed errors from the dependency array of a useMemo, but the memoization dependencies should reflect all values used within the memoized computation. Verify that removing errors doesn't break the error display logic.

5. System Design Considerations

Pattern Inconsistency: The useCallback usage across files follows different patterns:

• Some files wrap all async functions in useCallback
• Others only wrap functions passed as dependencies
• Dependency arrays vary in completeness

Recommendation: Establish a consistent pattern. Consider:

// Option 1: Only memoize when passing to children or multiple useEffects
const loadData = useCallback(async () => {
  // implementation
}, [dependencies]);

// Option 2: Use useEffect with inline async function when only used once
useEffect(() => {
  async function loadData() {
    // implementation
  }
  loadData();
}, [dependencies]);

6. Missing Type Safety Improvements

While fixing lint errors, opportunities for better TypeScript usage were missed:

format-date.ts: Functions use string but could benefit from branded types for date strings:

type DateString = string & { __brand: 'DateString' };
type PostgreSQLTimestamp = string & { __brand: 'PostgreSQLTimestamp' };

Recommendations

Immediate Actions:

1. Review all useCallback dependency arrays for completeness
2. Remove commented dead code in inline-event.tsx
3. Verify removed dependencies don't break functionality (especially errors in sidepane.tsx)

System-Level Improvements:

1. Establish useCallback conventions in your style guide
2. Consider extracting data fetching into custom hooks for consistency
3. Add TypeScript branded types for date handling to prevent bugs

Future Considerations:

1. The current pattern of individual async functions in components might benefit from a data fetching abstraction layer
2. Consider implementing proper error boundaries for async operations
3. The commented code patterns suggest feature flag or gradual rollout needs - consider implementing proper feature toggles

Verdict

This PR successfully resolves the immediate lint errors, but the fixes reveal some underlying architectural patterns that could benefit from standardization. The changes are safe to merge, but I recommend addressing the consistency and dead code removal in a follow-up PR.

Severity: Minor to Medium - Safe to merge with follow-up improvements recommended.

---

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.

If this PR is still relevant:

• Rebase it on the latest main branch
• Add a comment explaining its current status
• Request a review if it's ready

Thank you for your contributions!