fix(windows): address 15+ review comments from ellipsis/greptile/coderabbit

Applies fixes for review comments on PR #383 (Windows build support).
All fixes verified on macOS build + 51 tests passing.

Critical bug fixes:
- tests/test_extraction.cpp: fix infinite recursion in compat_mkdir
  (#else branch called itself, causing stack overflow on macOS/Linux).
- rac_platform_compat.h: fix rac_to_wstring to use MultiByteToWideChar
  (previous byte-widening corrupted UTF-8 multi-byte paths).
- onnx_embedding_provider.cpp: use rac_to_wstring helper instead of
  local broken byte-widening; include platform_compat header.
- wakeword_onnx.cpp: store wstring in named locals before calling .c_str()
  to avoid dangling pointers to destroyed temporaries in Ort::Session ctors.
- download_orchestrator.cpp: mkdir_p now handles both '/' and '\' separators
  so Windows paths don't skip intermediate directories.
- .bat files: convert to CRLF line endings + add .gitattributes to enforce
  (LF-only caused cmd.exe parser failures at 512-byte boundaries).
- CMakeLists.txt: use generator expressions for MSVC Release flags (VS is
  multi-config, so CMAKE_BUILD_TYPE is empty at configure time).
- tests/CMakeLists.txt: guard Windows DLL copy with DEFINED check so
  empty onnxruntime_SOURCE_DIR doesn't produce bogus paths.

Test portability:
- test_extraction.cpp: remove_dir now uses rmdir /s /q on Windows, rm -rf
  elsewhere. create_temp_dir checks _mkdir return on Windows.
- test_download_orchestrator.cpp: mkdir_p uses `mkdir` on Windows instead
  of Unix-only `mkdir -p`.

Build script hygiene:
- build-windows.bat: disable RAG backend in default preset (broken on
  Windows currently); copy .dll runtime libs for --shared builds; exit
  non-zero when tests fail.
- CMakePresets.json: remove CMAKE_BUILD_TYPE from VS presets (ignored by
  multi-config generators); disable RAG consistently.

Docs/comments:
- CMakeLists.txt: document why C++20 is required (MSVC needs it for
  designated initializers used throughout the codebase).
- result_free.cpp: correct misleading MSVC /alternatename comment; the
  actual strategy is to exclude the file on MSVC via CMakeLists.
- rag_backend.h: remove unnecessary __attribute__((visibility("default")))
  — class is internal-only, never exposed in public headers.

Minor:
- telemetry_json.cpp: check gmtime_s/gmtime_r return value; emit raw epoch
  on failure instead of strftime'ing uninitialized struct tm.
- onnx_backend.cpp: prefer model.int8.onnx over model.onnx in TTS model
  discovery (mirrors the STT path already doing this).

Test plan:
- cmake -B build -DRAC_BUILD_TESTS=ON && cmake --build build -j8: clean
- test_extraction: 22/22 PASSED (proves compat_mkdir fix works)
- test_core: 13/13 PASSED
- test_download_orchestrator: 16/16 PASSED
- Total: 51/51 passing, 0 regressions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: sanchitmonga22

sdk/runanywhere-commons/.gitattributes

@@ -0,0 +1,9 @@
+# Windows batch files MUST use CRLF line endings. cmd.exe's parser can
+# misbehave at 512-byte boundaries with LF-only line endings (label parsing
+# breaks, labels with delayed expansion fail intermittently). Keep these
+# files as CRLF even on non-Windows checkouts.
+*.bat text eol=crlf
+*.cmd text eol=crlf
+
+# Shell scripts must stay LF.
+*.sh text eol=lf

sdk/runanywhere-commons/CMakeLists.txt

@@ -46,6 +46,10 @@ option(RAC_BUILD_SERVER "Build OpenAI-compatible HTTP server (runanywhere-server
 # C++ CONFIGURATION
 # =============================================================================
 
+# C++20 is required because MSVC does not implement C++17 designated initializers
+# (e.g. `struct S s = {.field = value};`) — the codebase uses them in several
+# places (service vtable registration, config structs). All supported compilers
+# (GCC 10+, Clang 11+, MSVC 19.28+ / VS 2019 16.8+) support C++20.
 set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
@@ -329,9 +333,14 @@ elseif(MSVC)
     # Suppress common harmless warnings in third-party code
     add_compile_options(/wd4244 /wd4267 /wd4996)
     add_compile_definitions(_CRT_SECURE_NO_WARNINGS NOMINMAX WIN32_LEAN_AND_MEAN)
-    if(CMAKE_BUILD_TYPE STREQUAL "Release")
-        add_compile_options(/O2 /DNDEBUG)
-    endif()
+    # Use generator expressions instead of CMAKE_BUILD_TYPE so Release flags
+    # apply correctly with multi-config generators (Visual Studio), where
+    # CMAKE_BUILD_TYPE is empty at configure time and the config is selected
+    # at build time via --config Release.
+    add_compile_options(
+        $<$<CONFIG:Release>:/O2>
+        $<$<CONFIG:Release>:/DNDEBUG>
+    )
 endif()
 
 # =============================================================================

sdk/runanywhere-commons/CMakePresets.json

@@ -42,17 +42,16 @@
         "RAC_BUILD_BACKENDS": "ON",
         "RAC_BACKEND_LLAMACPP": "ON",
         "RAC_BACKEND_ONNX": "ON",
-        "RAC_BACKEND_RAG": "ON",
+        "RAC_BACKEND_RAG": "OFF",
         "RAC_BACKEND_WHISPERCPP": "OFF",
         "RAC_BUILD_PLATFORM": "OFF",
-        "RAC_BUILD_TESTS": "ON",
-        "CMAKE_BUILD_TYPE": "Release"
+        "RAC_BUILD_TESTS": "ON"
       }
     },
     {
       "name": "windows-msvc-core",
       "displayName": "Windows MSVC (core only)",
-      "description": "Windows build with MSVC, core library only (no backends)",
+      "description": "Windows build with MSVC, core library only (no backends). Build config is selected at build time via --config Release (Visual Studio generators are multi-config and ignore CMAKE_BUILD_TYPE).",
       "binaryDir": "${sourceDir}/build/windows-core",
       "generator": "Visual Studio 17 2022",
       "architecture": {
@@ -62,8 +61,7 @@
       "cacheVariables": {
         "RAC_BUILD_BACKENDS": "OFF",
         "RAC_BUILD_PLATFORM": "OFF",
-        "RAC_BUILD_TESTS": "ON",
-        "CMAKE_BUILD_TYPE": "Release"
+        "RAC_BUILD_TESTS": "ON"
       }
     }
   ]

sdk/runanywhere-commons/include/rac/core/rac_platform_compat.h

@@ -6,6 +6,46 @@
  * can use dirent.h, S_ISDIR, S_ISREG, etc. without #ifdef clutter.
  *
  * On non-Windows platforms this header is a no-op passthrough.
+ *
+ * -----------------------------------------------------------------------------
+ * TODO(future): Move this shim out of the public include path.
+ * -----------------------------------------------------------------------------
+ * Flagged in PR #383 review (coderabbitai): this header currently lives under
+ * `include/rac/core/` which means any SDK consumer that pulls a commons public
+ * header transitively inherits *un-prefixed* global names — `DIR`, `dirent`,
+ * `opendir`, `readdir`, `closedir`, `strcasecmp`, `strncasecmp`, and the
+ * `S_IS*` / `S_IFLNK` macros. That:
+ *   1. Breaks the project's "all public symbols must be `rac_` prefixed" rule
+ *      (see `sdk/runanywhere-commons/CLAUDE.md`).
+ *   2. Can collide with a consumer's own dirent shim or the platform's real
+ *      headers if they include in a different order.
+ * Impact is Windows-only in practice (POSIX platforms just pass through to
+ * system headers), but it's still a leaky public contract.
+ *
+ * Options for the cleanup:
+ *   A) Move the implementation to `src/internal/rac_platform_compat.h` so it's
+ *      never installed / never visible to consumers. All current call sites
+ *      would need their `#include` path updated. This is the preferred fix.
+ *   B) Keep the header public but rename every exposed symbol to `rac_*`
+ *      (`rac_opendir`, `rac_readdir`, `rac_dirent`, `rac_strcasecmp`, …) and
+ *      update every call site. More invasive in source but keeps drop-in
+ *      POSIX-ish semantics; less aligned with the project rule.
+ *
+ * Current call sites to update (option A or B):
+ *   - src/features/vlm/vlm_component.cpp
+ *   - src/features/rag/onnx_embedding_provider.cpp
+ *   - src/features/result_free.cpp
+ *   - src/backends/onnx/onnx_backend.cpp
+ *   - src/backends/onnx/wakeword_onnx.cpp
+ *   - src/infrastructure/download/download_orchestrator.cpp
+ *   - src/infrastructure/extraction/rac_extraction.cpp
+ *   - src/infrastructure/telemetry/telemetry_json.cpp
+ *   - tests/test_extraction.cpp, tests/test_download_orchestrator.cpp, tests/test_common.h
+ *   - Any new Windows-facing file that uses opendir/stat/etc.
+ *
+ * Deferred because it's orthogonal to the "make Windows build work" goal.
+ * Deferring is safe: the pollution only manifests on Windows, and today no
+ * external consumer builds commons on Windows yet.
  */
 
 #ifndef RAC_PLATFORM_COMPAT_H
@@ -129,17 +169,33 @@ static inline int closedir(DIR* dir) {
 
 #ifdef _WIN32
 /**
- * Convert a UTF-8 std::string to std::wstring for Windows wide-char APIs.
- * Used by ONNX Runtime Session creation which requires wchar_t* on Windows.
+ * Convert a UTF-8 std::string to std::wstring (UTF-16) for Windows wide-char APIs.
+ * Uses MultiByteToWideChar so non-ASCII paths (Chinese, Japanese, accented chars)
+ * convert correctly — a plain byte-widening copy would corrupt multi-byte UTF-8
+ * sequences. Used by ONNX Runtime session creation which requires wchar_t*.
  */
 inline std::wstring rac_to_wstring(const std::string& s) {
-    return std::wstring(s.begin(), s.end());
+    if (s.empty()) return {};
+    int size = MultiByteToWideChar(CP_UTF8, 0, s.data(),
+                                   static_cast<int>(s.size()), nullptr, 0);
+    if (size <= 0) return {};
+    std::wstring out(static_cast<size_t>(size), L'\0');
+    MultiByteToWideChar(CP_UTF8, 0, s.data(), static_cast<int>(s.size()),
+                        &out[0], size);
+    return out;
 }
 inline std::wstring rac_to_wstring(const char* s) {
-    if (!s) return {};
-    return std::wstring(s, s + strlen(s));
+    if (!s || !*s) return {};
+    return rac_to_wstring(std::string(s));
 }
-/** On Windows, ONNX Runtime expects wchar_t* paths */
+/**
+ * On Windows, ONNX Runtime expects wchar_t* paths.
+ * NOTE: The macro returns a pointer into a temporary std::wstring. Callers MUST
+ * store the result in a named local before calling .c_str(), otherwise the
+ * pointer dangles at the end of the full-expression:
+ *     std::wstring wp = rac_to_wstring(p);
+ *     Ort::Session s(env, wp.c_str(), options);
+ */
 #define RAC_ORT_PATH(p) rac_to_wstring(p).c_str()
 #else
 /** On non-Windows, ONNX Runtime expects char* paths */

sdk/runanywhere-commons/scripts/build-windows.bat

@@ -168,11 +168,15 @@ cmake -B "%BUILD_DIR%" ^
     -DRAC_BACKEND_ONNX=%BUILD_ONNX% ^
     -DRAC_BACKEND_LLAMACPP=%BUILD_LLAMACPP% ^
     -DRAC_BACKEND_WHISPERCPP=OFF ^
-    -DRAC_BACKEND_RAG=ON ^
+    -DRAC_BACKEND_RAG=OFF ^
     -DRAC_BUILD_TESTS=%BUILD_TESTS% ^
     -DRAC_BUILD_SHARED=%BUILD_SHARED% ^
     -DRAC_BUILD_PLATFORM=OFF ^
     "%ROOT_DIR%"
+::
+:: NOTE: RAC_BACKEND_RAG is disabled here because the RAG backend has known
+:: Windows build issues (tracked separately). Enable it manually once those
+:: are fixed.
 
 if errorlevel 1 (
     echo [ERROR] CMake configure failed.
@@ -204,18 +208,26 @@ echo [DIST] Copying libraries to distribution directory...
 
 if "%BUILD_SHARED%"=="ON" (set "LIB_EXT=dll") else (set "LIB_EXT=lib")
 
-:: Core library
+:: Core library (copy .lib import lib + .dll runtime lib for shared builds)
 if exist "%BUILD_DIR%\Release\rac_commons.lib" (
     copy /y "%BUILD_DIR%\Release\rac_commons.lib" "%DIST_DIR%\" >nul
     echo [OK] Copied rac_commons.lib
 )
+if "%BUILD_SHARED%"=="ON" if exist "%BUILD_DIR%\Release\rac_commons.dll" (
+    copy /y "%BUILD_DIR%\Release\rac_commons.dll" "%DIST_DIR%\" >nul
+    echo [OK] Copied rac_commons.dll
+)
 
 :: ONNX backend
 if "%BUILD_ONNX%"=="ON" (
     if exist "%BUILD_DIR%\src\backends\onnx\Release\rac_backend_onnx.lib" (
         copy /y "%BUILD_DIR%\src\backends\onnx\Release\rac_backend_onnx.lib" "%DIST_DIR%\" >nul
         echo [OK] Copied rac_backend_onnx.lib
     )
+    if "%BUILD_SHARED%"=="ON" if exist "%BUILD_DIR%\src\backends\onnx\Release\rac_backend_onnx.dll" (
+        copy /y "%BUILD_DIR%\src\backends\onnx\Release\rac_backend_onnx.dll" "%DIST_DIR%\" >nul
+        echo [OK] Copied rac_backend_onnx.dll
+    )
 )
 
 :: LlamaCPP backend
@@ -224,6 +236,10 @@ if "%BUILD_LLAMACPP%"=="ON" (
         copy /y "%BUILD_DIR%\src\backends\llamacpp\Release\rac_backend_llamacpp.lib" "%DIST_DIR%\" >nul
         echo [OK] Copied rac_backend_llamacpp.lib
     )
+    if "%BUILD_SHARED%"=="ON" if exist "%BUILD_DIR%\src\backends\llamacpp\Release\rac_backend_llamacpp.dll" (
+        copy /y "%BUILD_DIR%\src\backends\llamacpp\Release\rac_backend_llamacpp.dll" "%DIST_DIR%\" >nul
+        echo [OK] Copied rac_backend_llamacpp.dll
+    )
 )
 
 :: Headers
@@ -262,6 +278,11 @@ if "%RUN_TESTS%"=="1" (
     echo ========================================
     echo  Test Results: !TESTS_PASSED! passed, !TESTS_FAILED! failed
     echo ========================================
+
+    if !TESTS_FAILED! GTR 0 (
+        echo [ERROR] !TESTS_FAILED! test suite^(s^) failed.
+        exit /b 1
+    )
 )
 
 :: =============================================================================

sdk/runanywhere-commons/src/backends/onnx/onnx_backend.cpp

@@ -841,7 +841,14 @@ bool ONNXTTS::load_model(const std::string& model_path, TTSModelType model_type,
     }
 
     if (S_ISDIR(path_stat.st_mode)) {
-        model_onnx_path = model_path + "/model.onnx";
+        // Prefer the quantized int8 model over the full-precision one when both exist.
+        const std::string int8_candidate = model_path + "/model.int8.onnx";
+        if (stat(int8_candidate.c_str(), &path_stat) == 0) {
+            model_onnx_path = int8_candidate;
+            RAC_LOG_DEBUG("ONNX.TTS", "Using int8 model: %s", model_onnx_path.c_str());
+        } else {
+            model_onnx_path = model_path + "/model.onnx";
+        }
         tokens_path = model_path + "/tokens.txt";
         lexicon_path = model_path + "/lexicon.txt";
 

sdk/runanywhere-commons/src/backends/onnx/wakeword_onnx.cpp

@@ -565,10 +565,18 @@ RAC_ONNX_API rac_result_t rac_wakeword_onnx_init_shared_models(
     std::lock_guard<std::mutex> lock(backend->mutex);
 
     try {
-        // Load melspectrogram model (required for proper pipeline)
+        // Load melspectrogram model (required for proper pipeline).
+        // Store the wstring in a named local so the wchar_t* outlives the ctor call
+        // (RAC_ORT_PATH returns a pointer into a temporary on Windows).
         if (melspec_model_path) {
+#ifdef _WIN32
+            std::wstring melspec_wpath = rac_to_wstring(melspec_model_path);
             backend->melspec_session = std::make_unique<Ort::Session>(
-                *backend->env, RAC_ORT_PATH(melspec_model_path), *backend->session_options);
+                *backend->env, melspec_wpath.c_str(), *backend->session_options);
+#else
+            backend->melspec_session = std::make_unique<Ort::Session>(
+                *backend->env, melspec_model_path, *backend->session_options);
+#endif
 
             // Get input/output names
             auto input_name = backend->melspec_session->GetInputNameAllocated(0, backend->allocator);
@@ -584,8 +592,14 @@ RAC_ONNX_API rac_result_t rac_wakeword_onnx_init_shared_models(
 
         // Load embedding model (required)
         if (embedding_model_path) {
+#ifdef _WIN32
+            std::wstring embedding_wpath = rac_to_wstring(embedding_model_path);
+            backend->embedding_session = std::make_unique<Ort::Session>(
+                *backend->env, embedding_wpath.c_str(), *backend->session_options);
+#else
             backend->embedding_session = std::make_unique<Ort::Session>(
-                *backend->env, RAC_ORT_PATH(embedding_model_path), *backend->session_options);
+                *backend->env, embedding_model_path, *backend->session_options);
+#endif
 
             // Get input/output names
             auto input_name = backend->embedding_session->GetInputNameAllocated(0, backend->allocator);
@@ -651,8 +665,14 @@ RAC_ONNX_API rac_result_t rac_wakeword_onnx_load_model(
         model.model_path = model_path;
         model.threshold = backend->global_threshold;
 
+#ifdef _WIN32
+        std::wstring model_wpath = rac_to_wstring(model_path);
+        model.session = std::make_unique<Ort::Session>(
+            *backend->env, model_wpath.c_str(), *backend->session_options);
+#else
         model.session = std::make_unique<Ort::Session>(
-            *backend->env, RAC_ORT_PATH(model_path), *backend->session_options);
+            *backend->env, model_path, *backend->session_options);
+#endif
 
         // Get input/output names
         auto input_name = model.session->GetInputNameAllocated(0, backend->allocator);

sdk/runanywhere-commons/src/features/rag/onnx_embedding_provider.cpp

@@ -6,6 +6,7 @@
 #include "onnx_embedding_provider.h"
 #include "rac/features/rag/ort_guards.h"
 #include "rac/core/rac_logger.h"
+#include "rac/core/rac_platform_compat.h"
 #include "../../backends/onnx/onnx_backend.h"
 
 #include <nlohmann/json.hpp>
@@ -1006,10 +1007,12 @@ class ONNXEmbeddingProvider::Impl {
             return false;
         }
 
-        // Load model with session options
+        // Load model with session options.
+        // On Windows, ONNX Runtime requires wchar_t* paths; use rac_to_wstring
+        // (UTF-8 → UTF-16 via MultiByteToWideChar) so non-ASCII paths work.
+        // Materialize to a named local so the wchar_t* doesn't dangle.
 #ifdef _WIN32
-        // ONNX Runtime on Windows requires wchar_t* paths
-        std::wstring wpath(model_path.begin(), model_path.end());
+        std::wstring wpath = rac_to_wstring(model_path);
         status_guard.reset(ort_api_->CreateSession(
             ort_env_,
             wpath.c_str(),

sdk/runanywhere-commons/src/features/rag/rag_backend.h

@@ -44,11 +44,11 @@ struct RAGBackendConfig {
  * Coordinates vector store, embeddings service, and LLM service for
  * retrieval-augmented generation. Thread-safe for all operations.
  */
-#if defined(_MSC_VER)
+// RAGBackend is an internal implementation class — it is only referenced from
+// translation units inside this library and is never exposed through a public
+// header. No visibility attribute is needed (and asymmetric visibility on
+// non-MSVC vs MSVC previously caused inconsistent ABI behavior).
 class RAGBackend {
-#else
-class __attribute__((visibility("default"))) RAGBackend {
-#endif
 public:
     /**
      * @brief Construct RAG pipeline with service handles

sdk/runanywhere-commons/src/features/result_free.cpp

@@ -13,8 +13,11 @@
 #include "rac/features/tts/rac_tts_types.h"
 #include "rac/features/embeddings/rac_embeddings_types.h"
 
-// MSVC does not support __attribute__((weak)).
-// Use /alternatename linker directive for weak symbol emulation on MSVC.
+// MSVC does not support __attribute__((weak)). On MSVC this whole file is
+// excluded from the build via CMakeLists.txt, and each service translation
+// unit provides its own strong definition of `rac_*_result_free`. On other
+// compilers the weak attribute lets backend-specific .cpp files override
+// these fallback definitions at link time.
 #ifdef _MSC_VER
 #define RAC_WEAK_SYMBOL
 #else