feat: enhance agent guidelines with comprehensive code review patterns

.github/copilot-instructions.md

@@ -103,6 +103,119 @@ This is a React-based monitoring and management interface for YDB clusters. The
 - Ensure all user-facing text is internationalized
 - Test with a local YDB instance when possible
 
+## MANDATORY Code Review Rules (From 435 PR Analysis)
+
+### Top 5 Issues to Prevent
+
+1. **Hardcoded Strings (42 occurrences)** - #1 issue
+   - NEVER use hardcoded strings for user-facing text
+   - Even dashes must use i18n: `i18n('context_no-data')` not `'–'`
+2. **Missing Tests (39 occurrences)**
+   - ALL new features must have tests
+   - Unit tests for components, E2E tests for features
+3. **Improper State Management (16 occurrences)**
+   - Use Redux selectors, not direct state access
+   - NEVER mutate state in RTK Query
+4. **Missing Loading States (12 occurrences)**
+   - ALL async operations must show loading indicators
+   - Use Loader and TableSkeleton components
+5. **Poor Error Handling (9 occurrences)**
+   - Use ResponseError component for API errors
+   - Clear errors on user input
+
+### NEVER Do These
+
+- Use mock data in production code
+- Make direct fetch/axios calls (use `window.api`)
+- Skip required API parameters
+- Create duplicate API calls
+- Use improper type names (API types need 'T' prefix)
+- Commit without running lint and typecheck
+
+### ALWAYS Do These
+
+- Test with real YDB backend: `docker run -dp 8765:8765 ghcr.io/ydb-platform/local-ydb:nightly`
+- Include `fields_required: -1` for sysinfo API calls
+- Make tables sortable, resizable with sticky headers
+- Clear errors on user input in forms
+- Use conventional commits with lowercase subjects
+
+### Specific API Requirements
+
+```typescript
+// Required for threads data (PR #2599)
+window.api.viewer.getSysInfo({
+  nodeId: nodeId,
+  fields_required: -1, // MANDATORY parameter
+});
+```
+
+### Review Priority Matrix
+
+| Priority | Issue             | Check For                       |
+| -------- | ----------------- | ------------------------------- |
+| P0       | Hardcoded strings | All text uses i18n()            |
+| P0       | Missing tests     | New features have test coverage |
+| P1       | Mock data         | Only real backend data used     |
+| P1       | API patterns      | window.api usage, no fetch()    |
+| P2       | Type naming       | API types prefixed with 'T'     |
+
+## Universal Code Review Standards
+
+Apply these standards consistently for ALL code reviews:
+
+### Backend & API Standards
+
+- NO mock data - always use real backend data
+- Verify all required API parameters are included
+- Check for duplicate API calls
+- Ensure proper error handling
+
+### UI/UX Standards
+
+- Tables must have sticky headers, be sortable and resizable
+- Proper data alignment in all UI components
+- Use existing patterns from the codebase
+- Loading states for all async operations
+
+### Code Quality Standards
+
+- Conventional commit format with lowercase subjects
+- No unnecessary files (test scripts, debug code)
+- No duplicate code or tests
+- Proper TypeScript types (API types prefixed with 'T')
+- Simplified event handler types
+
+### Testing Standards
+
+- All new features must have tests
+- Verify functionality with real YDB instance
+- Remove all console.logs and debug statements
+
+## Common Code Patterns to Flag
+
+```typescript
+// ❌ Hardcoded string
+{
+  value || '–';
+}
+
+// ✅ Internationalized
+{
+  value || i18n('context_no-data');
+}
+
+// ❌ Verbose event type
+(event: React.MouseEvent<HTMLButtonElement, MouseEvent>) =>
+  // ✅ Simplified
+  (event: React.MouseEvent<HTMLButtonElement>) =>
+    // ❌ Direct API call
+    fetch('/api/data');
+
+// ✅ Via window.api
+window.api.module.getData();
+```
+
 ## Debugging Helpers
 
 - `window.api` - Access API methods in browser console

AGENTS.md

@@ -123,6 +123,41 @@ REACT_APP_BACKEND=http://your-cluster:8765  # Single cluster mode
 - Commit messages must follow conventional commit format
 - Always run `npm run lint` and `npm run typecheck` to catch issues early
 
+## PR & Commit Standards
+
+### Conventional Commits (REQUIRED)
+
+Based on 435 code reviews, all commits and PR titles must follow this format:
+
+```
+feat: add new feature
+fix: resolve bug
+chore: update dependencies
+docs: update documentation
+test: add tests
+refactor: code improvements
+```
+
+**Critical**: Subject MUST be lowercase (no proper nouns, sentence-case, or upper-case)
+
+Examples:
+
+- ✅ `fix: update storage group count calculation`
+- ❌ `Fix: Update Storage Group Count`
+- ❌ `fix: Update API endpoints`
+
+### PR Checklist
+
+Before submitting a PR:
+
+- [ ] Title follows conventional commits with lowercase subject
+- [ ] No mock data in code (test with real backend)
+- [ ] Tested with real YDB backend: `docker run -dp 8765:8765 ghcr.io/ydb-platform/local-ydb:nightly`
+- [ ] No duplicate API calls
+- [ ] All strings internationalized
+- [ ] Run `npm run lint` and `npm run typecheck`
+- [ ] Add tests for new features
+
 ### UI Framework
 
 The project uses Gravity UI (@gravity-ui/uikit) as the primary component library. When adding new UI components, prefer using Gravity UI components over custom implementations.
@@ -150,10 +185,50 @@ The project uses Gravity UI (@gravity-ui/uikit) as the primary component library
 
 All API calls go through `window.api` global object with domain-specific modules (viewer, schema, storage, etc.)
 
+**Critical API Parameters (from PR reviews):**
+
+```typescript
+// For sysinfo calls - REQUIRED for threads data (PR #2599)
+await window.api.viewer.getSysInfo({
+  nodeId: nodeId,
+  fields_required: -1, // Must include this parameter
+});
+
+// Common API mistakes:
+// ❌ Direct fetch calls
+fetch('/api/endpoint');
+
+// ❌ Missing required parameters
+window.api.viewer.getSysInfo({nodeId});
+
+// ✅ Correct pattern
+window.api.viewer.getSysInfo({nodeId, fields_required: -1});
+```
+
 ### Table Implementation
 
 Use `PaginatedTable` component for data grids with virtual scrolling. Tables require columns, fetchData function, and a unique tableName.
 
+**Table Requirements (from PR reviews by @Raubzeug and @artemmufazalov):**
+
+```typescript
+// ✅ Correct table implementation
+<ResizeableDataTable
+  columns={columns}
+  data={data}
+  settings={TABLE_SETTINGS}
+  theme="yandex-cloud"
+  stickyHeader      // Required for scrollable content
+  sortable          // Required for all tables
+  // Virtual scrolling enabled by default
+/>
+
+// Common table issues:
+// - Missing sticky headers (flagged in PR #2577)
+// - Incorrect sorting implementation (PR #2633)
+// - Not using ResizeableDataTable for data tables
+```
+
 ### Redux Toolkit Query Pattern
 
 API endpoints are injected using RTK Query's `injectEndpoints` pattern. Queries wrap `window.api` calls and provide hooks with loading states, error handling, and caching.
@@ -197,6 +272,20 @@ All user-facing text must be internationalized using the i18n system. Follow the
 - **NEVER** use hardcoded strings in UI components
 - **ALWAYS** create i18n entries for all user-visible text
 
+**Common i18n Mistakes (from 42 PR reviews):**
+
+```typescript
+// ❌ Hardcoded dash
+{
+  row.UserSID || '–';
+}
+
+// ✅ Internationalized
+{
+  row.UserSID || i18n('context_no-data');
+}
+```
+
 ### Performance Considerations
 
 - Tables use virtual scrolling for large datasets
@@ -259,6 +348,35 @@ Build artifacts are placed in `/build` directory. For embedded deployments, file
 - `PUBLIC_URL` - Base URL for static assets (use `.` for relative paths)
 - `GENERATE_SOURCEMAP` - Set to `false` for production builds
 
+## Common Issues from Code Reviews (Based on 435 PR Reviews)
+
+### Top 5 Issues to Avoid
+
+1. **Hardcoded Strings (42 occurrences)**
+
+   - ALWAYS use i18n for ALL user-facing text
+   - Even dashes: Use `i18n('context_no-data')` not `'–'`
+
+2. **Missing Tests (39 occurrences)**
+
+   - Add unit tests for new components
+   - E2E tests for new features
+   - Never merge without test coverage
+
+3. **Improper State Management (16 occurrences)**
+
+   - Use Redux selectors, not direct state access
+   - Never mutate state in RTK Query
+
+4. **Missing Loading States (12 occurrences)**
+
+   - Always show loading indicators
+   - Handle skeleton states for tables
+
+5. **Poor Error Handling (9 occurrences)**
+   - Use ResponseError component
+   - Clear errors on user input
+
 ## Common Issues & Troubleshooting
 
 ### Development Issues
@@ -273,13 +391,76 @@ Build artifacts are placed in `/build` directory. For embedded deployments, file
 1. **CORS errors**: Check if backend allows localhost:3000 origin in development
 2. **Authentication failures**: Verify credentials and authentication method
 3. **Network timeouts**: Check backend availability and network configuration
+4. **Double API calls**: Check for duplicate useEffect hooks or missing dependencies
 
 ### Performance Issues
 
 1. **Large table rendering**: Tables use virtual scrolling - ensure `PaginatedTable` is used
 2. **Bundle size**: Run `npm run analyze` to identify large dependencies
 3. **Memory leaks**: Check for missing cleanup in useEffect hooks
 
+## Real Code Review Examples & Solutions
+
+### Component Patterns (From Actual Reviews)
+
+#### Event Handler Types (PR #2642)
+
+```typescript
+// ❌ Verbose - Flagged in review
+const handleClick = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
+
+// ✅ Simplified - Approved
+const handleClick = (event: React.MouseEvent<HTMLButtonElement>) => {
+```
+
+#### Double API Calls (PR #2599)
+
+```typescript
+// ❌ Causes double requests - Flagged by @adameat
+useEffect(() => {
+  fetchData();
+}, []);
+
+useEffect(() => {
+  fetchData();
+}, [someParam]);
+
+// ✅ Single effect with proper dependencies
+useEffect(() => {
+  fetchData();
+}, [someParam]);
+```
+
+#### Mock Data Usage (PR #2599)
+
+```typescript
+// ❌ Using mock data - Always flagged
+const mockData = { threads: [...] };
+return mockData;
+
+// ✅ Always use real backend data
+const response = await window.api.viewer.getSysInfo({
+  nodeId,
+  fields_required: -1
+});
+return response;
+```
+
+### UI/UX Requirements (From Reviews)
+
+#### Progress Indicators (PR #2588 by @adameat)
+
+- 100% progress should always be displayed in green
+- Replication progress must show only when disk is not replicated
+- Proper alignment required in tooltips
+
+#### Table Features (PR #2577 by @Raubzeug)
+
+- Sticky headers for all scrollable tables
+- Sortable columns (especially numeric data)
+- Resizable columns
+- Use common patterns from existing tables
+
 ## Reference Resources
 
 - **YDB Documentation**: https://ydb.tech/en/docs/