fix: enhance configuration validation and error reporting

tsurumi-yizhou · tsurumi-yizhou · commit 5a54c6f35084 · 2026-04-14T18:49:17.000+08:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -17,6 +17,14 @@ option(CLORE_ENABLE_LTO "Enable ThinLTO for all targets" ON)
 option(CLORE_OFFLINE_BUILD "Disable network downloads during configuration" OFF)
 option(CLORE_ENABLE_TEST "Build unit tests" ON)
 
+if(WIN32 AND (MSVC OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND
+                      (CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "MSVC" OR
+                       CMAKE_CXX_COMPILER_LINKER_FRONTEND_VARIANT STREQUAL "MSVC"))))
+    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL" CACHE STRING
+        "Use the release CRT to stay ABI-compatible with the bundled Windows LLVM artifacts."
+        FORCE)
+endif()
+
 # Global flags that apply to all targets (including FetchContent dependencies).
 if(NOT MSVC)
     add_compile_options(-ffunction-sections -fdata-sections)
diff --git a/src/config/load.cppm b/src/config/load.cppm
@@ -1,5 +1,6 @@
 module;
 
+#include <algorithm>
 #include <cstdint>
 #include <expected>
 #include <filesystem>
@@ -99,10 +100,12 @@ auto to_config(RawTaskConfig&& raw) -> std::expected<TaskConfig, ConfigError> {
 }
 
 auto reject_forbidden_keys(const ::toml::table& table) -> std::expected<void, ConfigError> {
-    constexpr std::string_view forbidden_top_level_keys[] = {
+    constexpr std::string_view cli_backed_keys[] = {
         "compile_commands_path",
         "project_root",
         "output_root",
+    };
+    constexpr std::string_view removed_sections[] = {
         "validation",
         "navigation",
         "builtin",
@@ -114,11 +117,29 @@ auto reject_forbidden_keys(const ::toml::table& table) -> std::expected<void, Co
         "section_order",
     };
 
-    for(auto key : forbidden_top_level_keys) {
-        if(table.contains(key)) {
+    auto contains_key = [](const auto& keys, std::string_view key) {
+        for(auto candidate : keys) {
+            if(candidate == key) {
+                return true;
+            }
+        }
+        return false;
+    };
+
+    for(const auto& [key, _] : table) {
+        if(key.str().empty()) {
+            continue;
+        }
+
+        auto key_name = std::string_view{key.str()};
+        if(contains_key(cli_backed_keys, key_name)) {
             return std::unexpected(ConfigError{
                 .message = std::format("configuration key '{}' is not supported; use CLI arguments instead",
-                                       key)});
+                                       key_name)});
+        }
+        if(contains_key(removed_sections, key_name)) {
+            return std::unexpected(ConfigError{
+                .message = std::format("configuration key '{}' is no longer supported", key_name)});
         }
     }
 
diff --git a/src/extract/extract.cppm b/src/extract/extract.cppm
@@ -651,15 +651,29 @@ auto extract_project(const config::TaskConfig& config)
                 continue;
             }
 
-            auto symbol_id = sym.id;
+            auto normalized_sym = sym;
+            normalized_sym.declaration_location.file = owner_path.generic_string();
+            if(normalized_sym.definition_location.has_value() &&
+               !normalized_sym.definition_location->file.empty()) {
+                auto definition_path_result = resolve_path_under_directory(
+                    normalized_sym.definition_location->file,
+                    entry.directory);
+                if(!definition_path_result.has_value()) {
+                    return std::unexpected(std::move(definition_path_result.error()));
+                }
+                normalized_sym.definition_location->file =
+                    definition_path_result->lexically_normal().generic_string();
+            }
+
+            auto symbol_id = normalized_sym.id;
 
             auto sym_it = model.symbols.find(symbol_id);
             if(sym_it == model.symbols.end()) {
-                auto [inserted_it, _] = model.symbols.emplace(symbol_id, sym);
+                auto [inserted_it, _] = model.symbols.emplace(symbol_id, std::move(normalized_sym));
                 sym_it = inserted_it;
                 ++symbols_kept;
             } else {
-                merge_symbol_info(sym_it->second, sym);
+                merge_symbol_info(sym_it->second, std::move(normalized_sym));
             }
         }
 
diff --git a/src/generate/evidence.cppm b/src/generate/evidence.cppm
@@ -119,6 +119,32 @@ auto collect_facts(const extract::ProjectModel& model,
     return facts;
 }
 
+template <typename... Groups>
+auto collect_merged_facts(const extract::ProjectModel& model,
+                          std::uint32_t max_count,
+                          const std::string& project_root,
+                          const Groups&... groups) -> std::vector<SymbolFact> {
+    std::vector<SymbolFact> facts;
+    std::unordered_set<extract::SymbolID> seen;
+
+    auto append_group = [&](const auto& ids) {
+        for(const auto& id : ids) {
+            if(facts.size() >= max_count) {
+                return;
+            }
+            if(!seen.insert(id).second) {
+                continue;
+            }
+            if(auto* sym = lookup(model, id)) {
+                facts.push_back(to_symbol_fact(*sym, project_root));
+            }
+        }
+    };
+
+    (append_group(groups), ...);
+    return facts;
+}
+
 auto collect_summaries(const PageSummaryCache& cache,
                        const std::vector<std::string>& keys,
                        std::uint32_t max_count) -> std::vector<std::string> {
@@ -403,9 +429,11 @@ auto build_evidence_for_function_declaration_summary(
     pack.target_facts.push_back(to_symbol_fact(target, root));
 
     // Callers — how this function is used
-    pack.reverse_usage_context = collect_facts(model, target.called_by, rules.max_callers, root);
-    auto refs = collect_facts(model, target.referenced_by, rules.max_callers, root);
-    for(auto& f : refs) pack.reverse_usage_context.push_back(std::move(f));
+    pack.reverse_usage_context = collect_merged_facts(model,
+                                                      rules.max_callers,
+                                                      root,
+                                                      target.called_by,
+                                                      target.referenced_by);
 
     // Callees — what it delegates to (helps understand contract)
     pack.dependency_context = collect_facts(model, target.calls, rules.max_callees, root);
@@ -489,14 +517,15 @@ auto build_evidence_for_type_declaration_summary(
     pack.target_facts.push_back(to_symbol_fact(target, root));
 
     // Callers / references — how this type is used
-    pack.reverse_usage_context = collect_facts(model, target.called_by, rules.max_callers, root);
-    auto refs = collect_facts(model, target.referenced_by, rules.max_callers, root);
-    for(auto& f : refs) pack.reverse_usage_context.push_back(std::move(f));
+    pack.reverse_usage_context = collect_merged_facts(model,
+                                                      rules.max_callers,
+                                                      root,
+                                                      target.called_by,
+                                                      target.referenced_by,
+                                                      target.derived);
 
     // Base types and derived types
     pack.dependency_context = collect_facts(model, target.bases, rules.max_callees, root);
-    auto derived = collect_facts(model, target.derived, rules.max_callers, root);
-    for(auto& f : derived) pack.reverse_usage_context.push_back(std::move(f));
 
     // Sibling types as local context
     if(!target.enclosing_namespace.empty()) {
diff --git a/src/generate/generate.cppm b/src/generate/generate.cppm
@@ -245,6 +245,28 @@ auto extract_summary_from_prompt_output(const std::string& output) -> std::strin
     return clore::support::ensure_utf8(output);
 }
 
+auto summary_cache_key_for_request(const PagePlan& plan, const PromptRequest& request)
+    -> std::optional<std::string> {
+    switch(request.kind) {
+        case PromptKind::NamespaceSummary:
+        case PromptKind::ModuleSummary:
+            if(!request.target_key.empty()) {
+                return request.target_key;
+            }
+            if(!plan.owner_keys.empty()) {
+                return plan.owner_keys.front();
+            }
+            return std::nullopt;
+        case PromptKind::FunctionDeclarationSummary:
+        case PromptKind::TypeDeclarationSummary:
+            if(!request.target_key.empty()) {
+                return request.target_key;
+            }
+            return std::nullopt;
+        default: return std::nullopt;
+    }
+}
+
 auto render_generated_pages(const PagePlan& plan,
                             const config::TaskConfig& config,
                             const extract::ProjectModel& model,
@@ -268,25 +290,20 @@ auto update_summary_cache(PageSummaryCache& summaries,
                           const std::vector<PromptRequest>& prompt_requests,
                           const std::unordered_map<std::string, std::string>& prompt_outputs)
     -> void {
-    if(plan.owner_keys.empty()) {
-        return;
-    }
-
     for(const auto& request: prompt_requests) {
-        if(!is_page_summary_prompt(request.kind)) {
+        auto cache_key = summary_cache_key_for_request(plan, request);
+        if(!cache_key.has_value()) {
             continue;
         }
-        auto key = prompt_request_key(request);
-        auto it = prompt_outputs.find(key);
+        auto it = prompt_outputs.find(prompt_request_key(request));
         if(it == prompt_outputs.end()) {
             continue;
         }
         auto summary = extract_summary_from_prompt_output(it->second);
         if(summary.empty()) {
             continue;
         }
-        summaries[plan.owner_keys.front()] = std::move(summary);
-        return;
+        summaries[*cache_key] = std::move(summary);
     }
 }
 
diff --git a/src/network/openai.cppm b/src/network/openai.cppm
@@ -2330,28 +2330,27 @@ auto LLMClient::submit(std::uint64_t tag, std::string prompt)
     {
         std::lock_guard guard(active_run_mutex_);
         active_run = active_run_;
-    }
-
-    if(active_run != nullptr) {
-        {
-            std::lock_guard guard(active_run->mutex);
-            if(active_run->shutdown || active_run->stop_requested) {
-                return {};
-            }
-
-            active_run->pending.push_back(PendingRequest{
+        if(active_run == nullptr) {
+            pending_.push_back(PendingRequest{
                 .tag = tag,
                 .prompt = std::move(prompt),
             });
+            return {};
         }
-        active_run->condition.notify_one();
-        return {};
     }
 
-    pending_.push_back(PendingRequest{
-        .tag = tag,
-        .prompt = std::move(prompt),
-    });
+    {
+        std::lock_guard guard(active_run->mutex);
+        if(active_run->shutdown || active_run->stop_requested) {
+            return {};
+        }
+
+        active_run->pending.push_back(PendingRequest{
+            .tag = tag,
+            .prompt = std::move(prompt),
+        });
+    }
+    active_run->condition.notify_one();
     return {};
 }
 
@@ -2360,19 +2359,18 @@ auto LLMClient::request_stop() noexcept -> void {
     {
         std::lock_guard guard(active_run_mutex_);
         active_run = active_run_;
-    }
-
-    if(active_run != nullptr) {
-        {
-            std::lock_guard guard(active_run->mutex);
-            active_run->stop_requested = true;
-            active_run->pending.clear();
+        if(active_run == nullptr) {
+            pending_.clear();
+            return;
         }
-        active_run->condition.notify_all();
-        return;
     }
 
-    pending_.clear();
+    {
+        std::lock_guard guard(active_run->mutex);
+        active_run->stop_requested = true;
+        active_run->pending.clear();
+    }
+    active_run->condition.notify_all();
 }
 
 auto LLMClient::compute_retry_delay(std::uint32_t next_attempt) const
@@ -2384,8 +2382,11 @@ auto LLMClient::run(Callback on_complete) -> std::expected<void, LLMError> {
     if(configuration_error_.has_value()) {
         return std::unexpected(*configuration_error_);
     }
-    if(pending_.empty()) {
-        return {};
+    {
+        std::lock_guard guard(active_run_mutex_);
+        if(pending_.empty()) {
+            return {};
+        }
     }
 
     auto environment = detail::read_environment();
@@ -2394,11 +2395,13 @@ auto LLMClient::run(Callback on_complete) -> std::expected<void, LLMError> {
     }
 
     auto run_state = std::make_shared<RunState>();
-    run_state->pending = std::move(pending_);
-    pending_.clear();
-
     {
         std::lock_guard guard(active_run_mutex_);
+        if(pending_.empty()) {
+            return {};
+        }
+        run_state->pending = std::move(pending_);
+        pending_.clear();
         active_run_ = run_state;
     }
 
diff --git a/tests/unit/config/config.cpp b/tests/unit/config/config.cpp
@@ -99,6 +99,9 @@ exclude = ["test/.*", "build/.*"]
 compile_commands_path = "/tmp/compile_commands.json"
 )" );
         EXPECT_FALSE(result.has_value());
+        ASSERT_FALSE(result.has_value());
+        EXPECT_EQ(result.error().message,
+              "configuration key 'compile_commands_path' is not supported; use CLI arguments instead");
     }
 
     TEST_CASE(load_rejects_project_root) {
@@ -121,6 +124,9 @@ output_root = "/tmp/output"
 index = true
 )" );
         EXPECT_FALSE(result.has_value());
+        ASSERT_FALSE(result.has_value());
+        EXPECT_EQ(result.error().message,
+              "configuration key 'page_types' is no longer supported");
     }
 
     TEST_CASE(load_from_invalid_toml) {
@@ -170,6 +176,9 @@ fail_on_empty_section = true
 fail_on_h1_in_output = true
 )" );
         EXPECT_FALSE(result.has_value());
+        ASSERT_FALSE(result.has_value());
+        EXPECT_EQ(result.error().message,
+              "configuration key 'validation' is no longer supported");
     }
 
     TEST_CASE(load_rejects_removed_navigation_section) {
diff --git a/tests/unit/extract/extract.cpp b/tests/unit/extract/extract.cpp
@@ -155,6 +155,18 @@ void Widget::set_value(int v) { value_ = v; }
         EXPECT_TRUE(found_multiply);
         EXPECT_TRUE(found_widget);
 
+        auto math_h = (temp_dir / "src" / "math.h").lexically_normal().generic_string();
+        auto math_cpp = (temp_dir / "src" / "math.cpp").lexically_normal().generic_string();
+        auto add_it = std::ranges::find_if(model.symbols, [](const auto& item) {
+            return item.second.qualified_name == "mylib::math::add";
+        });
+        ASSERT_TRUE(add_it != model.symbols.end());
+        EXPECT_EQ(add_it->second.declaration_location.file, math_h);
+        ASSERT_TRUE(add_it->second.definition_location.has_value());
+        EXPECT_EQ(add_it->second.definition_location->file, math_cpp);
+        EXPECT_TRUE(model.files.contains(add_it->second.declaration_location.file));
+        EXPECT_TRUE(model.files.contains(add_it->second.definition_location->file));
+
         fs::remove_all(temp_dir);
     }
 
diff --git a/tests/unit/generate/generate.cpp b/tests/unit/generate/generate.cpp
