TASK-053 validation iter1: address reviewer findings

- lookup_v2: move canonical path into cache_key to avoid second alloc;
  reuse key.path for all tier probes.
- route_cache::find_by_view: promote via bucket iterator directly,
  removing a redundant map_.find() on every cache hit.
- classify_route_tier: compile regexes with std::regex::nosubs since
  match-only is needed (capture groups were never consumed).
- bench_route_lookup: hoist the cache-hit query path to a static so
  the timed region measures the lookup, not std::string construction.
- v2_dispatch_contract_test: replace fixed 50 ms sleep with
  wait_for_server_ready helper; add is_prefix assertion on the 405
  method-mismatch path; cover this gap explicitly.
- route_table_test: add cache_find_by_view_promotes_to_front to pin
  LRU promotion semantics of find_by_view.
- webserver_impl.hpp: refresh the lookup_v2 doc-comment now that it
  is the sole dispatch-path resolver post-step-3 (the prior NOTE
  described the pre-cutover shadow-table state).

Also capture the iter1 unworked-review-issues report under
specs/unworked_review_issues/.

Author: etr

specs/unworked_review_issues/2026-05-29_112007_task-053.md

@@ -141,7 +141,7 @@ webserver_impl::lookup_result
 webserver_impl::lookup_v2(http_method method, const std::string& path) {
     lookup_result result;
 
-    const std::string lookup_path = canonicalize_lookup_path(path);
+    std::string lookup_path = canonicalize_lookup_path(path);
 
     // Step 1: cache. Cache under the canonical key so /foo and /foo/
     // share an entry. Use find_by_view to avoid copying lookup_path
@@ -155,16 +155,17 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
         result.captured_params = std::move(cached.captured_params);
         return result;
     }
-    // Construct key only on the miss path, where we need to own the
-    // string for insertion into the cache.
-    cache_key key{method, lookup_path};
+    // Construct key by moving lookup_path into the cache_key, avoiding a
+    // second heap allocation. All subsequent tier lookups use key.path,
+    // which is the same std::string now owned by the key.
+    cache_key key{method, std::move(lookup_path)};
 
     // Step 2: walk the tiers under a shared lock.
     {
         std::shared_lock table_lock(route_table_mutex_);
 
         // 2a. Exact tier — single hash probe.
-        auto exact_it = exact_routes_.find(lookup_path);
+        auto exact_it = exact_routes_.find(key.path);
         if (exact_it != exact_routes_.end()) {
             result.found = true;
             result.tier = tier_hit::exact;
@@ -175,7 +176,7 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
         // 2b. Radix tier — segment-trie walk.
         if (!result.found) {
             radix_match<route_entry> rm;
-            if (param_and_prefix_routes_.find(lookup_path, rm) && rm.entry) {
+            if (param_and_prefix_routes_.find(key.path, rm) && rm.entry) {
                 result.found = true;
                 result.tier = tier_hit::radix;
                 result.entry = *rm.entry;
@@ -188,7 +189,7 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
         // and on_methods_), so no compilation cost is paid per lookup.
         if (!result.found) {
             for (const auto& rr : regex_routes_) {
-                if (std::regex_match(lookup_path, rr.compiled_re)) {
+                if (std::regex_match(key.path, rr.compiled_re)) {
                     result.found = true;
                     result.tier = tier_hit::regex;
                     result.entry = rr.entry;

src/detail/webserver_dispatch.cpp

@@ -149,12 +149,12 @@ class route_cache {
             return false;
         }
         std::size_t b = bucket_hash % map_.bucket_count();
-        for (auto it = map_.cbegin(b), end = map_.cend(b); it != end; ++it) {
+        for (auto it = map_.begin(b), end = map_.end(b); it != end; ++it) {
             if (it->first.method == method && it->first.path == path) {
-                // Promote: splice requires a mutable iterator from the main map.
-                auto main_it = map_.find(it->first);
-                list_.splice(list_.begin(), list_, main_it->second);
-                out = main_it->second->second;
+                // Promote using the mutable bucket iterator directly —
+                // avoids a second map_.find() call on every cache hit.
+                list_.splice(list_.begin(), list_, it->second);
+                out = it->second->second;
                 return true;
             }
         }

src/httpserver/detail/route_cache.hpp

@@ -88,7 +88,8 @@ inline route_tier_result classify_route_tier(const detail::http_endpoint& idx) {
         std::regex re;
         try {
             re = std::regex(idx.get_url_normalized(),
-                            std::regex::extended | std::regex::icase);
+                            std::regex::extended | std::regex::icase
+                            | std::regex::nosubs);
         } catch (const std::regex_error& e) {
             throw std::invalid_argument(
                 std::string("invalid regex route pattern '")

src/httpserver/detail/route_tier.hpp

@@ -245,12 +245,12 @@ class webserver_impl {
     // documented above. Returns lookup_result; populates `tier` even on
     // miss (tier_hit::none) so callers can branch deterministically.
     //
-    // NOTE (CWE-284 guard): lookup_v2 is a SHADOW TABLE only. The live
-    // HTTP dispatch path (finalize_answer -> resolve_resource_for_request)
-    // still uses the v1 registered_resources* maps. lookup_v2 is used
-    // by tests to pin the v2 tier pipeline and will be wired into
-    // finalize_answer at Cycle K dispatch cutover. Until then, any access
-    // control registered via the v2 table has NO effect on live traffic.
+    // NOTE (TASK-053): lookup_v2 is the live dispatch path. As of
+    // TASK-053 step 3, resolve_resource_for_request() calls lookup_v2()
+    // exclusively; all v1 helper functions were deleted. The v1
+    // registered_resources* maps survive only as registration-side
+    // bookkeeping (lambda/class conflict detection and WebSocket dispatch
+    // — see the 'Registration-side resource maps' section below).
     lookup_result lookup_v2(http_method method, const std::string& path);
 
     // TASK-045 -- Lifecycle hook bus (skeleton, no firing yet).

src/httpserver/detail/webserver_impl.hpp

@@ -195,19 +195,21 @@ int main() {
     auto* impl = hs::webserver_test_access::impl(*ws);
 
     // ----- (a) cache-hit -----
+    // Pre-allocate the query path outside the timed region so the bench
+    // measures only the lookup machinery, not std::string construction.
+    static const std::string kCacheHitPath("/api/v1/users/me");
+
     // First call warms the cache. Subsequent calls hit it.
     {
-        auto warm = impl->lookup_v2(
-            hs::http_method::get, std::string("/api/v1/users/me"));
+        auto warm = impl->lookup_v2(hs::http_method::get, kCacheHitPath);
         do_not_optimize(warm);
     }
 
     std::printf("bench_route_lookup (a): cache-hit (/api/v1/users/me)\n");
     const double median_cache_ns = measure_median_ns(
         "cache-hit",
         [&]() {
-            auto r = impl->lookup_v2(
-                hs::http_method::get, std::string("/api/v1/users/me"));
+            auto r = impl->lookup_v2(hs::http_method::get, kCacheHitPath);
             do_not_optimize(r);
         },
         OUTER, INNER_CACHE);

test/bench_route_lookup.cpp

@@ -336,6 +336,28 @@ LT_BEGIN_AUTO_TEST(route_table_suite, radix_tree_remove_one_terminus_does_not_cl
     LT_CHECK(m.is_prefix_match);
 LT_END_AUTO_TEST(radix_tree_remove_one_terminus_does_not_clear_sibling)
 
+// find_by_view must promote the hit to the front of the LRU list,
+// just like find(). After a find_by_view hit on /a, inserting /d into a
+// capacity-3 cache must evict the true LRU entry (/b), not /a.
+LT_BEGIN_AUTO_TEST(route_table_suite, cache_find_by_view_promotes_to_front)
+    htd::route_cache cache(3);
+    htd::cache_value v;
+    cache.insert({ht::http_method::get, "/a"}, v);
+    cache.insert({ht::http_method::get, "/b"}, v);
+    cache.insert({ht::http_method::get, "/c"}, v);
+    // Promote /a to front via find_by_view.
+    htd::cache_value out;
+    std::string_view pv_a{"/a"};
+    LT_CHECK(cache.find_by_view(ht::http_method::get, pv_a, out));
+    // Insert /d -> evicts oldest non-A entry (/b).
+    cache.insert({ht::http_method::get, "/d"}, v);
+    htd::cache_value probe;
+    LT_CHECK(cache.find({ht::http_method::get, "/a"}, probe));
+    LT_CHECK(!cache.find({ht::http_method::get, "/b"}, probe));
+    LT_CHECK(cache.find({ht::http_method::get, "/c"}, probe));
+    LT_CHECK(cache.find({ht::http_method::get, "/d"}, probe));
+LT_END_AUTO_TEST(cache_find_by_view_promotes_to_front)
+
 LT_BEGIN_AUTO_TEST_ENV()
     AUTORUN_TESTS()
 LT_END_AUTO_TEST_ENV()

test/unit/route_table_test.cpp

@@ -54,6 +54,30 @@
 #include "./httpserver.hpp"
 #include "./littletest.hpp"
 
+// wait_for_server_ready: poll the given port with an HTTP GET / until the
+// server responds (any status), or until the deadline elapses. This avoids
+// a fixed sleep that is either too short on slow/sanitizer CI builds or
+// wastes 50 ms of wall time on every test invocation.
+static void wait_for_server_ready(int port,
+                                  std::chrono::milliseconds deadline
+                                      = std::chrono::milliseconds(3000)) {
+    using clock = std::chrono::steady_clock;
+    auto end = clock::now() + deadline;
+    std::string url = "http://127.0.0.1:" + std::to_string(port) + "/";
+    while (clock::now() < end) {
+        CURL* c = curl_easy_init();
+        if (!c) break;
+        curl_easy_setopt(c, CURLOPT_URL, url.c_str());
+        curl_easy_setopt(c, CURLOPT_NOBODY, 1L);
+        curl_easy_setopt(c, CURLOPT_CONNECTTIMEOUT_MS, 50L);
+        curl_easy_setopt(c, CURLOPT_TIMEOUT_MS, 50L);
+        CURLcode rc = curl_easy_perform(c);
+        curl_easy_cleanup(c);
+        if (rc == CURLE_OK) return;
+        std::this_thread::sleep_for(std::chrono::milliseconds(5));
+    }
+}
+
 using httpserver::create_webserver;
 using httpserver::hook_phase;
 using httpserver::http_request;
@@ -148,7 +172,7 @@ LT_BEGIN_AUTO_TEST(v2_dispatch_contract_suite, parameterized_route_extracts_capt
     auto resource = std::make_shared<echo_id_resource>();
     ws.register_path("/users/{id}", resource);
     ws.start(false);
-    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    wait_for_server_ready(PORT);
 
     response_capture r = do_get("/users/42");
     ws.stop();
@@ -182,7 +206,7 @@ LT_BEGIN_AUTO_TEST(v2_dispatch_contract_suite, prefix_route_marks_is_prefix_true
     auto resource = std::make_shared<hello_resource>();
     ws.register_prefix("/static", resource);
     ws.start(false);
-    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    wait_for_server_ready(PORT);
 
     response_capture r = do_get("/static/foo/bar");
     ws.stop();
@@ -217,7 +241,7 @@ LT_BEGIN_AUTO_TEST(v2_dispatch_contract_suite, exact_route_marks_is_prefix_false
     auto resource = std::make_shared<hello_resource>();
     ws.register_path("/exact", resource);
     ws.start(false);
-    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    wait_for_server_ready(PORT);
 
     response_capture r = do_get("/exact");
     ws.stop();
@@ -243,6 +267,8 @@ LT_BEGIN_AUTO_TEST(v2_dispatch_contract_suite, method_mismatch_still_resolves_ro
                 if (ctx.matched.has_value()) {
                     probe.last_matched_engaged.store(true,
                                                      std::memory_order_relaxed);
+                    probe.last_is_prefix.store(ctx.matched->is_prefix,
+                                               std::memory_order_relaxed);
                     probe.last_resource_non_null.store(
                         ctx.resource != nullptr,
                         std::memory_order_relaxed);
@@ -259,14 +285,17 @@ LT_BEGIN_AUTO_TEST(v2_dispatch_contract_suite, method_mismatch_still_resolves_ro
     resource->set_allowing(httpserver::http_method::get, true);
     ws.register_path("/get_only", resource);
     ws.start(false);
-    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    wait_for_server_ready(PORT);
 
     long post_status = do_post_status("/get_only");
     ws.stop();
 
     LT_CHECK_EQ(post_status, 405L);
     LT_CHECK(probe.last_matched_engaged.load());
     LT_CHECK(probe.last_resource_non_null.load());
+    // /get_only is an exact (non-prefix) route; is_prefix must be false
+    // in the 405 code path as well as in the 200 code path.
+    LT_CHECK_EQ(probe.last_is_prefix.load(), false);
 LT_END_AUTO_TEST(method_mismatch_still_resolves_route)
 
 LT_BEGIN_AUTO_TEST_ENV()