perf(ci): Optimize CI pipeline and resolve code quality issues

Optimize CI/CD pipeline with comprehensive caching and improve code quality
by resolving all clang-tidy warnings.

CI/CD Optimizations:
- Add LLVM/Clang 18 installation caching to reduce setup time
- Add Homebrew package caching for macOS builds
- Improve Docker build cache scoping for better reusability
- Optimize macOS workflow to run after Linux tests complete
- Remove redundant format checks from macOS workflow

Code Quality Improvements:
- Fix clang-tidy warnings for unused variables and parameters
- Eliminate magic numbers by introducing named constants (kBytesPerMB)
- Resolve variable shadowing in RAII guard classes
- Fix dead store warnings with NOLINT annotations where appropriate
- Add move constructors/operators delete for RAII guards
- Improve code formatting consistency

Bug Fixes:
- Initialize variables to prevent undefined behavior
- Add missing #include <memory> in mygram-cli.cpp
- Make ExtractAllFilters static as it doesn't use instance state
- Fix potential overflow in result sorting with proper casting

Author: libraz

.github/workflows/ci.yml

@@ -70,14 +70,20 @@ jobs:
         restore-keys: |
           ${{ runner.os }}-ccache-
 
+    - name: Cache LLVM installation
+      id: cache-llvm
+      uses: actions/cache@v4
+      with:
+        path: |
+          /usr/lib/llvm-18
+          /usr/bin/clang-format-18
+          /usr/bin/clang-tidy-18
+        key: ${{ runner.os }}-llvm-18
+
     - name: Install dependencies
       run: |
         sudo apt-get update
         sudo apt-get install -y \
-          wget \
-          lsb-release \
-          software-properties-common \
-          gnupg \
           cmake \
           build-essential \
           libmysqlclient-dev \
@@ -87,11 +93,16 @@ jobs:
           lcov \
           ccache
 
-        # Install LLVM/Clang 18 for clang-format and clang-tidy
+    - name: Install LLVM/Clang 18
+      if: steps.cache-llvm.outputs.cache-hit != 'true'
+      run: |
         wget https://apt.llvm.org/llvm.sh
         chmod +x llvm.sh
         sudo ./llvm.sh 18
         sudo apt-get install -y clang-format-18 clang-tidy-18
+
+    - name: Setup LLVM alternatives
+      run: |
         sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-18 100
         sudo update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-18 100
 
@@ -171,8 +182,8 @@ jobs:
 
   build-and-test-macos:
     name: Build and Test (macOS)
-    needs: changes
-    if: github.event_name == 'pull_request' && (needs.changes.outputs.src == 'true' || needs.changes.outputs.tests == 'true')
+    needs: [changes, linux-test-and-coverage]
+    if: needs.changes.outputs.src == 'true' || needs.changes.outputs.tests == 'true'
     runs-on: macos-latest
 
     steps:
@@ -195,6 +206,14 @@ jobs:
         restore-keys: |
           ${{ runner.os }}-ccache-
 
+    - name: Cache Homebrew packages
+      uses: actions/cache@v4
+      with:
+        path: ~/Library/Caches/Homebrew
+        key: ${{ runner.os }}-brew-${{ hashFiles('.github/workflows/ci.yml') }}
+        restore-keys: |
+          ${{ runner.os }}-brew-
+
     - name: Install dependencies (macOS)
       run: |
         brew install \
@@ -203,14 +222,8 @@ jobs:
           icu4c \
           readline \
           pkg-config \
-          llvm@18 \
           ccache
 
-    - name: Check code formatting
-      run: |
-        export PATH="/opt/homebrew/opt/llvm@18/bin:$PATH"
-        make CLANG_FORMAT=clang-format format-check
-
     - name: Setup ccache
       run: |
         ccache --set-config=max_size=500M
@@ -264,8 +277,6 @@ jobs:
 
   docker-build:
     name: Docker Build Test
-    needs: changes
-    if: github.event_name == 'pull_request' && needs.changes.outputs.docker == 'true'
     runs-on: ubuntu-latest
 
     steps:
@@ -283,8 +294,8 @@ jobs:
         push: false
         load: true
         tags: mygramdb:test
-        cache-from: type=gha
-        cache-to: type=gha,mode=max
+        cache-from: type=gha,scope=docker-build
+        cache-to: type=gha,mode=max,scope=docker-build
 
     - name: Test Docker image
       run: |

.github/workflows/docker-release.yml

@@ -68,8 +68,10 @@ jobs:
         push: true
         tags: ${{ steps.meta.outputs.tags }}
         labels: ${{ steps.meta.outputs.labels }}
-        cache-from: type=gha
-        cache-to: type=gha,mode=max
+        cache-from: |
+          type=gha,scope=docker-build
+          type=gha,scope=docker-release
+        cache-to: type=gha,mode=max,scope=docker-release
 
     - name: Generate release notes
       id: release_notes

src/cli/mygram-cli.cpp

@@ -10,6 +10,7 @@
 
 #include <cstring>
 #include <iostream>
+#include <memory>
 #include <sstream>
 #include <string>
 #include <utility>

src/config/config.cpp

@@ -571,8 +571,9 @@ Config ParseConfigFromJson(const json& root) {
       config.cache.enabled = cache["enabled"].get<bool>();
     }
     if (cache.contains("max_memory_mb")) {
+      constexpr size_t kBytesPerMB = 1024 * 1024;  // Bytes in one megabyte
       int max_memory_mb = cache["max_memory_mb"].get<int>();
-      config.cache.max_memory_bytes = static_cast<size_t>(max_memory_mb) * 1024 * 1024;
+      config.cache.max_memory_bytes = static_cast<size_t>(max_memory_mb) * kBytesPerMB;
     }
     if (cache.contains("min_query_cost_ms")) {
       config.cache.min_query_cost_ms = cache["min_query_cost_ms"].get<double>();
@@ -603,9 +604,10 @@ Config ParseConfigFromJson(const json& root) {
     if (config.cache.enabled && config.cache.max_memory_bytes > 0) {
       auto system_info = utils::GetSystemMemoryInfo();
       if (system_info) {
-        constexpr double kMaxCacheRatio = 0.5;  // Maximum 50% of physical memory
-        uint64_t max_allowed_cache = static_cast<uint64_t>(
-            static_cast<double>(system_info->total_physical_bytes) * kMaxCacheRatio);
+        constexpr double kMaxCacheRatio = 0.5;       // Maximum 50% of physical memory
+        constexpr size_t kBytesPerMB = 1024 * 1024;  // Bytes in one megabyte
+        auto max_allowed_cache =
+            static_cast<uint64_t>(static_cast<double>(system_info->total_physical_bytes) * kMaxCacheRatio);
 
         if (config.cache.max_memory_bytes > max_allowed_cache) {
           std::stringstream err_msg;
@@ -614,12 +616,11 @@ Config ParseConfigFromJson(const json& root) {
           err_msg << "  Physical memory: " << utils::FormatBytes(system_info->total_physical_bytes) << "\n";
           err_msg << "  Maximum allowed (50% of physical memory): " << utils::FormatBytes(max_allowed_cache) << "\n";
           err_msg << "  Recommendation:\n";
-          err_msg << "    - Set cache.max_memory_mb to at most "
-                  << (max_allowed_cache / 1024 / 1024) << " MB\n";
+          err_msg << "    - Set cache.max_memory_mb to at most " << (max_allowed_cache / kBytesPerMB) << " MB\n";
           err_msg << "    - Consider system memory requirements for index and operations\n";
           err_msg << "  Example:\n";
           err_msg << "    cache:\n";
-          err_msg << "      max_memory_mb: " << (max_allowed_cache / 1024 / 1024);
+          err_msg << "      max_memory_mb: " << (max_allowed_cache / kBytesPerMB);
           throw std::runtime_error(err_msg.str());
         }
       } else {

src/config/config.h

@@ -243,7 +243,7 @@ struct LoggingConfig {
  */
 struct CacheConfig {
   bool enabled = true;                          ///< Enable/disable cache (default: true)
-  size_t max_memory_bytes = 32 * 1024 * 1024;  ///< Maximum cache memory in bytes (default: 32MB)  // NOLINT
+  size_t max_memory_bytes = 32 * 1024 * 1024;   ///< Maximum cache memory in bytes (default: 32MB)  // NOLINT
   double min_query_cost_ms = 10.0;              ///< Minimum query cost to cache (default: 10ms)  // NOLINT
   int ttl_seconds = 3600;                       ///< Cache entry TTL (default: 1 hour, 0 = no TTL)  // NOLINT
   std::string invalidation_strategy = "ngram";  ///< Invalidation strategy: "ngram", "table"

src/config/config_help.cpp

@@ -208,9 +208,10 @@ nlohmann::json ConfigToJson(const Config& config) {
   };
 
   // Cache configuration
+  constexpr size_t kBytesPerMB = 1024 * 1024;  // Bytes in one megabyte
   json["cache"] = {
       {"enabled", config.cache.enabled},
-      {"max_memory_mb", config.cache.max_memory_bytes / (1024 * 1024)},  // Convert bytes to MB for display
+      {"max_memory_mb", config.cache.max_memory_bytes / kBytesPerMB},  // Convert bytes to MB for display
       {"min_query_cost_ms", config.cache.min_query_cost_ms},
       {"ttl_seconds", config.cache.ttl_seconds},
       {"invalidation_strategy", config.cache.invalidation_strategy},

src/index/index.cpp

@@ -455,10 +455,12 @@ void Index::Optimize(uint64_t total_docs) {
   // RAII guard to ensure flag is cleared
   struct OptimizationGuard {
     std::atomic<bool>& flag;
-    explicit OptimizationGuard(std::atomic<bool>& f) : flag(f) {}
+    explicit OptimizationGuard(std::atomic<bool>& flag_ref) : flag(flag_ref) {}
     ~OptimizationGuard() { flag = false; }
     OptimizationGuard(const OptimizationGuard&) = delete;
     OptimizationGuard& operator=(const OptimizationGuard&) = delete;
+    OptimizationGuard(OptimizationGuard&&) = delete;
+    OptimizationGuard& operator=(OptimizationGuard&&) = delete;
   };
   OptimizationGuard guard(is_optimizing_);
 

src/mysql/binlog_reader.cpp

@@ -70,14 +70,16 @@ bool BinlogReader::Start() {
   struct RunningGuard {
     std::atomic<bool>& flag;
     bool& success;
-    explicit RunningGuard(std::atomic<bool>& f, bool& s) : flag(f), success(s) {}
+    explicit RunningGuard(std::atomic<bool>& flag_ref, bool& success_ref) : flag(flag_ref), success(success_ref) {}
     ~RunningGuard() {
       if (!success) {
         flag = false;
       }
     }
     RunningGuard(const RunningGuard&) = delete;
     RunningGuard& operator=(const RunningGuard&) = delete;
+    RunningGuard(RunningGuard&&) = delete;
+    RunningGuard& operator=(RunningGuard&&) = delete;
   };
   bool start_success = false;
   RunningGuard guard(running_, start_success);
@@ -156,6 +158,7 @@ bool BinlogReader::Start() {
     reader_thread_ = std::make_unique<std::thread>(&BinlogReader::ReaderThreadFunc, this);
 
     spdlog::info("Binlog reader started from GTID: {}", current_gtid_);
+    // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) - Used by RunningGuard destructor
     start_success = true;  // Mark start as successful
     return true;
   } catch (const std::exception& e) {
@@ -277,6 +280,7 @@ void BinlogReader::ReaderThreadFunc() {
         continue;
       }
       spdlog::info("[binlog worker] Reconnected successfully");
+      // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) - Used in next iteration after continue
       reconnect_attempt = 0;  // Reset delay counter after successful reconnection
       continue;
     }
@@ -337,6 +341,7 @@ void BinlogReader::ReaderThreadFunc() {
       if (!binlog_connection_->Connect()) {
         spdlog::error("Failed to reconnect: {}", binlog_connection_->GetLastError());
       } else {
+        // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) - Used in next iteration after continue
         reconnect_attempt = 0;  // Reset delay counter after successful reconnection
       }
       continue;
@@ -392,6 +397,7 @@ void BinlogReader::ReaderThreadFunc() {
           if (!binlog_connection_->Connect()) {
             spdlog::error("Failed to reconnect: {}", binlog_connection_->GetLastError());
           } else {
+            // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) - Used after break exits inner loop
             reconnect_attempt = 0;  // Reset delay counter after successful reconnection
           }
           break;  // Exit inner loop to retry from outer loop
@@ -527,7 +533,7 @@ bool BinlogReader::EvaluateRequiredFilters(const std::unordered_map<std::string,
 }
 
 std::unordered_map<std::string, storage::FilterValue> BinlogReader::ExtractAllFilters(
-    const RowData& row_data, const config::TableConfig& table_config) const {
+    const RowData& row_data, const config::TableConfig& table_config) {
   std::unordered_map<std::string, storage::FilterValue> all_filters;
 
   // Convert required_filters to FilterConfig format for extraction
@@ -598,7 +604,7 @@ bool BinlogReader::CompareFilterValue(const storage::FilterValue& value, const c
   } else if (std::holds_alternative<double>(value)) {
     // Float comparison
     double val = std::get<double>(value);
-    double target;
+    double target = 0.0;
     try {
       target = std::stod(filter.value);
     } catch (const std::exception& e) {
@@ -656,7 +662,7 @@ bool BinlogReader::CompareFilterValue(const storage::FilterValue& value, const c
     // For datetime comparison, we need to parse target value
     // For now, assume target is numeric (epoch timestamp)
     // TODO: Add proper datetime parsing if needed
-    uint64_t target;
+    uint64_t target = 0;
     try {
       target = std::stoull(filter.value);
     } catch (const std::exception& e) {
@@ -1087,7 +1093,7 @@ std::optional<BinlogEvent> BinlogReader::ParseBinlogEvent(const unsigned char* b
       event.type = BinlogEventType::UPDATE;
       event.table_name = table_meta->table_name;
       event.primary_key = after_row.primary_key;
-      event.text = after_row.text;  // New text (after image)
+      event.text = after_row.text;       // New text (after image)
       event.old_text = before_row.text;  // Old text (before image) for index update
       event.gtid = current_gtid_;
 
@@ -1488,14 +1494,14 @@ bool BinlogReader::FetchColumnNames(TableMetadata& metadata) {
 
   // Cache miss or stale: use SHOW COLUMNS (faster than INFORMATION_SCHEMA)
   // Escape backticks in identifier names
-  auto escape_identifier = [](const std::string& id) {
+  auto escape_identifier = [](const std::string& identifier) {
     std::string escaped;
-    escaped.reserve(id.length());
-    for (char c : id) {
-      if (c == '`') {
+    escaped.reserve(identifier.length());
+    for (char chr : identifier) {
+      if (chr == '`') {
         escaped += "``";  // Double backtick for escaping
       } else {
-        escaped += c;
+        escaped += chr;
       }
     }
     return escaped;

src/mysql/binlog_reader.h

@@ -48,7 +48,7 @@ struct BinlogEvent {
   BinlogEventType type = BinlogEventType::UNKNOWN;
   std::string table_name;
   std::string primary_key;
-  std::string text;  // Normalized text for INSERT/UPDATE (after image for UPDATE)
+  std::string text;      // Normalized text for INSERT/UPDATE (after image for UPDATE)
   std::string old_text;  // Before image text for UPDATE events (empty for INSERT/DELETE)
   std::unordered_map<std::string, storage::FilterValue> filters;
   std::string gtid;  // GTID for this event
@@ -254,8 +254,8 @@ class BinlogReader {
    * @param table_config Table configuration to use for filter extraction
    * @return Map of filter name to FilterValue
    */
-  std::unordered_map<std::string, storage::FilterValue> ExtractAllFilters(
-      const RowData& row_data, const config::TableConfig& table_config) const;
+  static std::unordered_map<std::string, storage::FilterValue> ExtractAllFilters(
+      const RowData& row_data, const config::TableConfig& table_config);
 
   /**
    * @brief Extract all filter columns using default table_config_ (single-table mode)

src/mysql/connection.cpp

@@ -418,7 +418,7 @@ bool Connection::ValidateUniqueColumn(const std::string& database, const std::st
     return false;
   }
 
-  int count;
+  int count = 0;
   try {
     count = std::stoi(row[0]);
   } catch (const std::invalid_argument& e) {