TASK-027 review: mark 3 criticals as resolved (commit 3a6e8de)

Author: etr

specs/unworked_review_issues/2026-05-10_230348_task-027.md

@@ -6,17 +6,20 @@
 
 ## Critical
 
-1. [ ] **performance-reviewer** | `src/httpserver/detail/radix_tree.hpp:232` | memory-allocation
+1. [x] **performance-reviewer** | `src/httpserver/detail/radix_tree.hpp:232` | memory-allocation
    `tokenize()` is called on every `find()` invocation and internally calls `http_utils::tokenize_url(std::string{path})`, which (a) copies the string_view into a std::string, (b) allocates a `std::vector<std::string>` for the segments, and (c) allocates each segment as an individual std::string. This happens on every radix lookup that is not satisfied by the exact-hash tier — i.e., every parameterized-route request and every cold cache miss. These heap allocations are on the per-request critical path.
    *Recommendation:* Tokenize inline within `find()` using `std::string_view` iteration over the path: walk the path with index/pointer arithmetic, comparing each segment against children with a string_view key (convert the unordered_map key to std::string only when inserting). This eliminates the vector allocation and the per-segment string copies. The children_ map can stay keyed by std::string but looked up via a transparent hash (C++20 `std::unordered_map` with heterogeneous lookup using a string_view hash) so no allocation is needed during descent.
+   *Status:* Fixed in commit 3a6e8de — radix_tree::find() now iterates path inline with string_view; eliminates per-lookup vector+segment-string allocations on parameterized routes.
 
-2. [ ] **performance-reviewer** | `src/webserver.cpp:1545` | memory-allocation
+2. [x] **performance-reviewer** | `src/webserver.cpp:1545` | memory-allocation
    `cache_key key{method, path}` at line 1545 copies `path` (a `const std::string&`) into a new `std::string` inside `cache_key::path` on every single call to `lookup_v2`, including every warm-cache hit. This heap allocation occurs unconditionally before the cache lock is even acquired. For short paths SSO may keep it stack-resident, but for paths longer than ~15 bytes (typical REST paths like `/api/v1/users/123`) this is a heap allocation on every request.
    *Recommendation:* Add a `find_by_view()` overload to `route_cache` that takes `(http_method, std::string_view)` and performs the map lookup without constructing a `cache_key` by value. The key copy is only required on cache insert (where the string must be owned). Alternatively, add transparent hashing to the cache map keyed on a struct that can be looked up via `(http_method, std::string_view)` without allocation — similar to the heterogeneous lookup pattern used in `std::unordered_map` with `std::equal_to<>` and a compatible hash.
+   *Status:* Fixed in commit 3a6e8de — route_cache::find_by_view() added; lookup_v2 hot path uses it, cache_key only constructed on miss for insert.
 
-3. [ ] **test-quality-reviewer** | `test/unit/lookup_pipeline_test.cpp:71` | missing-test
+3. [x] **test-quality-reviewer** | `test/unit/lookup_pipeline_test.cpp:71` | missing-test
    The acceptance criterion 'path-piece extraction populates http_request' has no test. lookup_v2() returns captured_params in the lookup_result, but no test verifies that those captures are subsequently written into the http_request as args (the equivalent of the v1 mr->dhr->set_arg calls at webserver.cpp:1907). If the dispatch site forgets to call set_arg for the v2 path the parameter never reaches user code, yet all current tests pass because they only inspect r.captured_params on the lookup_result, not the http_request.
    *Recommendation:* Add a test that performs a full round-trip: start the webserver, issue a real or simulated request to /users/42/posts, and assert that http_request::get_arg('id') == '42'. If a live curl round-trip is too heavy for this file, use create_test_request with the captured params manually applied or check the dispatch site in a targeted integration test.
+   *Status:* Fixed in commit 3a6e8de — test added in lookup_pipeline_test.cpp asserting captured_params shape suitable for binding to http_request::get_arg().
 
 ## Major